From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751471AbdEPGkO (ORCPT ); Tue, 16 May 2017 02:40:14 -0400 Received: from mx2.suse.de ([195.135.220.15]:43914 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751027AbdEPGkM (ORCPT ); Tue, 16 May 2017 02:40:12 -0400 Subject: Re: [PATCH 03/18] xen/pvcalls: initialize the module and register the xenbus backend To: Stefano Stabellini , xen-devel@lists.xen.org Cc: linux-kernel@vger.kernel.org, boris.ostrovsky@oracle.com, Stefano Stabellini References: <1494880570-14209-1-git-send-email-sstabellini@kernel.org> <1494880570-14209-3-git-send-email-sstabellini@kernel.org> From: Juergen Gross Message-ID: Date: Tue, 16 May 2017 08:40:05 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <1494880570-14209-3-git-send-email-sstabellini@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15/05/17 22:35, Stefano Stabellini wrote: > The pvcalls backend has one ioworker per cpu: the ioworkers are > implemented as a cpu bound workqueue, and will deal with the actual > socket and data ring reads/writes. > > ioworkers are global: we only have one set for all the frontends. They > process requests on their wqs list in order, once they are done with a > request, they'll remove it from the list. A spinlock is used for > protecting the list. Each ioworker is bound to a different cpu to > maximize throughput. > > Signed-off-by: Stefano Stabellini > CC: boris.ostrovsky@oracle.com > CC: jgross@suse.com > --- > drivers/xen/pvcalls-back.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 64 insertions(+) > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c > index 2dbf7d8..46a889a 100644 > --- a/drivers/xen/pvcalls-back.c > +++ b/drivers/xen/pvcalls-back.c > @@ -25,6 +25,26 @@ > #include > #include > > +struct pvcalls_ioworker { > + struct work_struct register_work; > + atomic_t io; > + struct list_head wqs; > + spinlock_t lock; > + int num; > +}; > + > +struct pvcalls_back_global { > + struct pvcalls_ioworker *ioworkers; > + int nr_ioworkers; > + struct workqueue_struct *wq; > + struct list_head privs; > + struct rw_semaphore privs_lock; > +} pvcalls_back_global; > + > +static void pvcalls_back_ioworker(struct work_struct *work) > +{ > +} > + > static int pvcalls_back_probe(struct xenbus_device *dev, > const struct xenbus_device_id *id) > { > @@ -59,3 +79,47 @@ static int pvcalls_back_uevent(struct xenbus_device *xdev, > .uevent = pvcalls_back_uevent, > .otherend_changed = pvcalls_back_changed, > }; > + > +static int __init pvcalls_back_init(void) > +{ > + int ret, i, cpu; > + > + if (!xen_domain()) > + return -ENODEV; > + > + ret = xenbus_register_backend(&pvcalls_back_driver); > + if (ret < 0) > + return ret; > + > + init_rwsem(&pvcalls_back_global.privs_lock); > + INIT_LIST_HEAD(&pvcalls_back_global.privs); > + pvcalls_back_global.wq = alloc_workqueue("pvcalls_io", 0, 0); > + if (!pvcalls_back_global.wq) > + goto error; > + pvcalls_back_global.nr_ioworkers = num_online_cpus(); Really? Recently I cam across a system with 640 dom0 cpus. I don't think we want 640 workers initialized when loading the backend module. I'd prefer one or a few workers per connected frontend. > + pvcalls_back_global.ioworkers = kzalloc( > + sizeof(*pvcalls_back_global.ioworkers) * > + pvcalls_back_global.nr_ioworkers, GFP_KERNEL); kcalloc()? > + if (!pvcalls_back_global.ioworkers) > + goto error; > + i = 0; > + for_each_online_cpu(cpu) { > + pvcalls_back_global.ioworkers[i].num = i; > + atomic_set(&pvcalls_back_global.ioworkers[i].io, 1); > + spin_lock_init(&pvcalls_back_global.ioworkers[i].lock); > + INIT_LIST_HEAD(&pvcalls_back_global.ioworkers[i].wqs); > + INIT_WORK(&pvcalls_back_global.ioworkers[i].register_work, > + pvcalls_back_ioworker); > + i++; > + } > + return 0; > + > +error: > + if (pvcalls_back_global.wq) > + destroy_workqueue(pvcalls_back_global.wq); > + xenbus_unregister_driver(&pvcalls_back_driver); > + kfree(pvcalls_back_global.ioworkers); > + memset(&pvcalls_back_global, 0, sizeof(pvcalls_back_global)); > + return -ENOMEM; > +} > +module_init(pvcalls_back_init); > Juergen