> -----Original Message----- > From: Sell, Timothy C > Sent: Wednesday, May 18, 2016 6:25 PM > To: 'Thomas Gleixner' > Cc: mi...@kernel.org; dave.han...@linux.intel.com; > ti...@freescale.com; ga...@kernel.crashing.org; Kershner, David A; > cor...@lwn.net; mi...@redhat.com; h...@zytor.com; Arfvidson, Erik; > hof...@osadl.org; dzic...@redhat.com; Curtin, Alexander Paul; > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > pra...@redhat.com; Binder, David Anthony; nhor...@redhat.com; > dan.j.willi...@intel.com; linux-ker...@vger.kernel.org; linux- > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > Maintainer; Greg KH; Jes Sorensen > Subject: RE: new driver for drivers/virt/? > > > -----Original Message----- > > From: Thomas Gleixner [mailto:t...@linutronix.de] > > Sent: Wednesday, May 18, 2016 6:12 PM > > To: Sell, Timothy C > > Cc: mi...@kernel.org; dave.han...@linux.intel.com; > > ti...@freescale.com; ga...@kernel.crashing.org; Kershner, David A; > > cor...@lwn.net; mi...@redhat.com; h...@zytor.com; Arfvidson, Erik; > > hof...@osadl.org; dzic...@redhat.com; Curtin, Alexander Paul; > > janani.rvchn...@gmail.com; sudipm.mukher...@gmail.com; > > pra...@redhat.com; Binder, David Anthony; nhor...@redhat.com; > > dan.j.willi...@intel.com; linux-ker...@vger.kernel.org; linux- > > d...@vger.kernel.org; driverdev-de...@linuxdriverproject.org; *S-Par- > > Maintainer; Greg KH; Jes Sorensen > > Subject: Re: new driver for drivers/virt/? > > > > On Wed, 18 May 2016, Sell, Timothy C wrote: > > > We have a bus driver currently in drivers/staging/unisys/visorbus/ that > > > we are trying to get out of staging and into the kernel proper. Since > > > "visorbus" is a driver to host a virtual bus presented to a Linux guest > > > in a hypervisor environment (refer to > > > drivers/staging/unisys/Documentation/overview.txt for more details), > > > Greg KH and Jes Sorensen have suggested the possibility that > drivers/virt/ > > > might be a good place for visorbus. But right now, we see that the only > > > driver under drivers/virt/ is the Freescale hypervisor environment, > which > > > made us wonder whether this was really the correct place. > > > > > > Would you have any guidance for us? > > > Our intent is to push our visorbus out of staging immediately following > > > the current merge window. > > > > What's the problem with Gregs and Jes suggestion? I don't see any. > > > > That's good; glad you agree with them. We just wanted to double-check > with those of you listed as maintainers of drivers/virt/. Thanks. > > > There is bigger fish to fry than the final place of this driver. I had just > > a > > peek at the staging code and there is enough stuff which wants to be > > cleaned > > up before moving anywhere. I don't have time to do a proper review now, > > but > > here are a few hints upfront: > > > > 1) Locking: > > > > visordriver_callback_lock: > > > > That should be a mutex, not a semaphore > > > > periodic_work->lock: > > > > Why is this a rw_lock if it's only locked with write_lock? And what's > > the purpose of this lock at all? > > > > 2) Memory barriers: > > > > Completely undocumented wmb()s without corresponding rmb()s to do > > obscure > > protection of that periodic work stuff. > >
tglx: Re wmb/rmb: I believe you must have been looking at an older version of our code in Linus' tree, rather than the latest version in Greg's staging-next tree. Reason is, Linus' tree only contains our source code thru 3/11 (i.e., see (http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/log/drivers/staging/unisys), and wmb() was removed in Greg's staging-next tree on 3/30 with commit 64938182e7836650feeb9b2b9dadd510ed4b0dad (https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/drivers/staging?h=staging-next&id=64938182e7836650feeb9b2b9dadd510ed4b0dad). We've made a LOT of changes in Greg's staging-next since 3/11. However, the other issues you bring up still look to be valid in our latest source code (in Greg's staging-next). Tim Sell > > 3) periodic_work: > > > > That set of functions is obscure. Especially visor_periodic_work_stop() > > makes me shudder. See also #2. > > > > That work->lock does not inspire my confidence further. > > > > 4) Exports: > > > > A gazillion of exports which are just wrappers around another set of > > exports > > > > 5) Function comments: > > > > Try to mimic kerneldoc comments, i.e. start with: /** > > but do not implement any of the kerneldoc requirements. > > > > We'll take a look at these. Thanks. > > Tim Sell > > > I'll try do find a time slot for a proper review of that thing, but don't > > expect that to happen in the next days. > > > > Thanks, > > > > tglx > > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
