From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A2980183CD6 for ; Wed, 6 Nov 2024 09:21:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730884914; cv=none; b=uhuRaA7XqQ3+m6nRcq0JdmtK2/3FmmFT54NQaA0DWB9wn0UHgEjOkoIf3mxIakt22fRRR6QfMweSvQIGRzIo2kNYguClRp7XhQsHmv9FIY7Gs3rlpTrRyjUPg5GjQA7jWTifmPC4tlCaxYTYz4xGdOlj9nOH1vVGBV+cqwEgsIg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730884914; c=relaxed/simple; bh=Pw+CI4bHxgFkgqeCayKSDMkesfgD/0zFa0vhtdOxMjc=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=AZ885mlGA8e3xnetfacY//YriFG4R5LrH4ik208MnpD0qVghKYwJi3PKCpY7MhYataLwQwhJSMofv1tk/vTOjcBoqfgvjr1sF1D6QnFn1OUNnkgFuL3eLfLakLoMeEw7vHtMXWN06xhY1HKa1Xs258IQUmlB9vGGpGhb+N7QsZc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=gcBnbDW7; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="gcBnbDW7" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1730884911; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=G+/r8y82t5LpcD8J2mPdZCtNa7f1NkKmPiS7dBnIVQM=; b=gcBnbDW7IEY8/2CGHxpyLHgQNpZUM4AzTAFiQ56aqcELbcMt8ZdrdndQz1jsm5jMJd/djv 0A0F9/kTsPjnyD/ED47XPO5gfW5MjF/M8PnG3n4n4jV6r1Nxu9GewHzs7aO7ADYUagYWGs yvLKOFpGdankpfR12LdHxiBAB4gnlkY= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-623-zNY5oHQRO-SUdo-NEQlMCw-1; Wed, 06 Nov 2024 04:21:50 -0500 X-MC-Unique: zNY5oHQRO-SUdo-NEQlMCw-1 X-Mimecast-MFC-AGG-ID: zNY5oHQRO-SUdo-NEQlMCw Received: by mail-ed1-f72.google.com with SMTP id 4fb4d7f45d1cf-5c934ceea1fso775410a12.2 for ; Wed, 06 Nov 2024 01:21:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730884909; x=1731489709; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=G+/r8y82t5LpcD8J2mPdZCtNa7f1NkKmPiS7dBnIVQM=; b=oIiB0dVoAn8LsW+kY3uRCQj2Mu3pOX3CFGjRRvSBDCCAbgGFDQF4jrniawS1KURN80 j8LZsd5I9fM20b6ngHq9tgkvHp9XtRUP3Ow1VToGTUs9mW+PUH9ePzWwqbPQvpL63rtF TDxvQir60MHjb/lJ4rlhSPb5NoJYatJGaKzLTTQeSyDkVcwvUEv5aNMBpJWA8BZMF4QC cw0NKapfisNSQNn9RL0ZCb8aPSP78gDnV8aMQn8Pxia6sHrvpfYT8lHw+BBiKYxZIbva a2d4lHD4vXMFRgvxzNrK58FqM8qz+j8LjfWdABogjm0Bo/qWsLZhDPZ7zudTINzfDgEJ HtBg== X-Forwarded-Encrypted: i=1; AJvYcCXJrkPnr78oeKlTuUzTd1WZApDHAOuYhuid1WxMWbPka8CVaezzG9WhHxJpXXBVMeO6QJHEQx8n9sfgk/M=@vger.kernel.org X-Gm-Message-State: AOJu0YyJlFQ6maVhoymSllezb4uqj35gWOC3Ex2rfWSxXYZiqEguvvcC JwrpovOHHaZQ6XhbnW+l1TtaD5ZGbohHXh45xj2K7ULS6XaPw+1x6NR2aw4GIO9rv6E9Ps1ZnoB yoxkSrrHkhk151l0R4T5WSzoMLnrB56fyO9YAPq1GnlJLKsTAEGsiZVF3wOspbvArvOYk41gF/X D3x0JFUTPSg299nsZ4lKmNlq9D4qSJ//x7WH8S X-Received: by 2002:a17:906:dc91:b0:a9a:4f78:c3 with SMTP id a640c23a62f3a-a9e3a5a0da3mr2632258666b.21.1730884909160; Wed, 06 Nov 2024 01:21:49 -0800 (PST) X-Google-Smtp-Source: AGHT+IHaDNg6wSMGmRW4F0u9QWcnzmzJVMDxrKOMNX8UWkiv8lV8hKePP9x5R6DeNMjld7k4M/mJKlB64IX5d/Yhcbk= X-Received: by 2002:a17:906:dc91:b0:a9a:4f78:c3 with SMTP id a640c23a62f3a-a9e3a5a0da3mr2632249266b.21.1730884907353; Wed, 06 Nov 2024 01:21:47 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20241105072642.898710-1-lulu@redhat.com> <20241105072642.898710-5-lulu@redhat.com> In-Reply-To: From: Cindy Lu Date: Wed, 6 Nov 2024 17:21:09 +0800 Message-ID: Subject: Re: [PATCH v3 4/9] vhost: Add kthread support in function vhost_worker_create To: Jason Wang Cc: mst@redhat.com, michael.christie@oracle.com, sgarzare@redhat.com, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Nov 5, 2024 at 5:36=E2=80=AFPM Jason Wang wro= te: > > On Tue, Nov 5, 2024 at 3:27=E2=80=AFPM Cindy Lu wrote: > > > > Restored the previous functions kthread_wakeup and kthread_stop. > > Also add a new structure, vhost_task_fn. The function vhost_worker_crea= te > > Will initializes this structure based on the value of inherit_owner. > > > > Signed-off-by: Cindy Lu > > --- > > drivers/vhost/vhost.c | 71 ++++++++++++++++++++++++++++++++++++------- > > drivers/vhost/vhost.h | 6 ++++ > > 2 files changed, 66 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > index e40cef3a1fa5..603b146fccc1 100644 > > --- a/drivers/vhost/vhost.c > > +++ b/drivers/vhost/vhost.c > > @@ -741,43 +741,92 @@ static void vhost_workers_free(struct vhost_dev *= dev) > > xa_destroy(&dev->worker_xa); > > } > > > > +static int vhost_task_wakeup_fn(void *vtsk) > > +{ > > + vhost_task_wake((struct vhost_task *)vtsk); > > + return 0; > > +} > > Let's have a newline between two functions. > will fix this > > +static int vhost_kthread_wakeup_fn(void *p) > > +{ > > + return wake_up_process((struct task_struct *)p); > > +} > > +static int vhost_task_stop_fn(void *vtsk) > > +{ > > + vhost_task_stop((struct vhost_task *)vtsk); > > + return 0; > > +} > > +static int vhost_kthread_stop_fn(void *k) > > +{ > > + return kthread_stop((struct task_struct *)k); > > +} > > + > > static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev) > > { > > struct vhost_worker *worker; > > - struct vhost_task *vtsk; > > + struct vhost_task *vtsk =3D NULL; > > + struct task_struct *task =3D NULL; > > char name[TASK_COMM_LEN]; > > int ret; > > u32 id; > > > > + /* Allocate resources for the worker */ > > worker =3D kzalloc(sizeof(*worker), GFP_KERNEL_ACCOUNT); > > if (!worker) > > return NULL; > > > > + worker->fn =3D kzalloc(sizeof(struct vhost_task_fn), GFP_KERNEL= _ACCOUNT); > > + if (!worker->fn) { > > + kfree(worker); > > + return NULL; > > + } > > + > > worker->dev =3D dev; > > snprintf(name, sizeof(name), "vhost-%d", current->pid); > > > > - vtsk =3D vhost_task_create(vhost_run_work_list, vhost_worker_ki= lled, > > - worker, name); > > - if (!vtsk) > > - goto free_worker; > > - > > mutex_init(&worker->mutex); > > init_llist_head(&worker->work_list); > > worker->kcov_handle =3D kcov_common_handle(); > > - worker->vtsk =3D vtsk; > > > > - vhost_task_start(vtsk); > > + if (dev->inherit_owner) { > > + /* Create and start a vhost task */ > > + vtsk =3D vhost_task_create(vhost_run_work_list, > > + vhost_worker_killed, worker, n= ame); > > + if (!vtsk) > > + goto free_worker; > > + > > + worker->vtsk =3D vtsk; > > + worker->fn->wakeup =3D vhost_task_wakeup_fn; > > + worker->fn->stop =3D vhost_task_stop_fn; > > + > > + vhost_task_start(vtsk); > > + } else { > > + /* Create and start a kernel thread */ > > + task =3D kthread_create(vhost_run_work_kthread_list, wo= rker, > > + "vhost-%d", current->pid); > > + if (IS_ERR(task)) { > > + ret =3D PTR_ERR(task); > > + goto free_worker; > > + } > > + worker->task =3D task; > > + worker->fn->wakeup =3D vhost_kthread_wakeup_fn; > > + worker->fn->stop =3D vhost_kthread_stop_fn; > > + > > + wake_up_process(task); > > + /* Attach to the vhost cgroup */ > > + ret =3D vhost_attach_cgroups(dev); > > + if (ret) > > + goto stop_worker; > > + } > > > > ret =3D xa_alloc(&dev->worker_xa, &id, worker, xa_limit_32b, GF= P_KERNEL); > > if (ret < 0) > > goto stop_worker; > > worker->id =3D id; > > - > > return worker; > > - > > stop_worker: > > - vhost_task_stop(vtsk); > > + worker->fn->stop(dev->inherit_owner ? (void *)vtsk : (void *)ta= sk); > > free_worker: > > + kfree(worker->fn); > > kfree(worker); > > return NULL; > > } > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > > index c650c4506c70..ebababa4e340 100644 > > --- a/drivers/vhost/vhost.h > > +++ b/drivers/vhost/vhost.h > > @@ -25,8 +25,13 @@ struct vhost_work { > > vhost_work_fn_t fn; > > unsigned long flags; > > }; > > +struct vhost_task_fn { > > + int (*wakeup)(void *task); > > Let's have comments to explain the semantics of each operation. > sure, will fix this > > + int (*stop)(void *task); > > +}; > > I think the goal is to reduce if/else, so while at this, let's > introduce more ops. For example the create_worker one? > sure, will change this part thanks cindy > > > > struct vhost_worker { > > + struct task_struct *task; > > struct vhost_task *vtsk; > > struct vhost_dev *dev; > > /* Used to serialize device wide flushing with worker swapping.= */ > > @@ -36,6 +41,7 @@ struct vhost_worker { > > u32 id; > > int attachment_cnt; > > bool killed; > > + struct vhost_task_fn *fn; > > }; > > > > /* Poll a file (eventfd or socket) */ > > -- > > 2.45.0 > > > > Thanks >