> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx] > Sent: Thursday, June 09, 2016 3:56 PM > To: Sell, Timothy C > Cc: corbet@xxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; Arfvidson, Erik; hofrat@xxxxxxxxx; > dzickus@xxxxxxxxxx; jes.sorensen@xxxxxxxxxx; Curtin, Alexander Paul; > janani.rvchndrn@xxxxxxxxx; sudipm.mukherjee@xxxxxxxxx; > prarit@xxxxxxxxxx; Binder, David Anthony; nhorman@xxxxxxxxxx; > dan.j.williams@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux- > doc@xxxxxxxxxxxxxxx; driverdev-devel@xxxxxxxxxxxxxxxxxxxxxx; *S-Par- > Maintainer; Kershner, David A > Subject: RE: [PATCH v4 09/29] staging: unisys: visorinput: remove > unnecessary locking > > On Thu, 9 Jun 2016, Sell, Timothy C wrote: > > > From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx] > > > > > > I think I asked this before, but I might have missed the answer. > > > > > > Why is this a rw_sempahore? It's never taken with down_read and > looking > > > at the usage sites it's simply a mutex, right? > > > > If the semaphore --> mutex change would have been as simple as it > sounds, > > we would have had NO problem including it with the next version (v3) of > this > > patchset. But unfortunately, this change uncovered a latent defect, which > > necessitated yet another patch. (I know... hard to believe that something > > this simple would do that, but it did.) Rather than further complicating > this > > patchset, we thought it would be better to address the visorinput issues > via a > > separate follow-on patchset. > > That makes me curious. What's the issue? Functional is the mutex the same > thing as the r/w semaphore when the latter is only taken down_write and > locked > and released by the same thread, which is the case as far as I can tell. > The issue: using it uninitialized (<blush>). A semaphore appears to let you get away with it, but a mutex does NOT. We had to shuffle some things around to get this right. If you're interested in a preview, you can find a patch in github at https://github.com/davidker/unisys/commit/039e6e517b4a17e2d135a9df85cc1e24a39c2670. The second bullet in that commit comment describes the scenario where we were attempting to access the lock in visorinput_open() before we had actually initialized it: * we canNOT get into visorinput_open() until the device structure is totally initialized, by delaying the input_register_device() until the end of device initialization I.e., before this patch, we WERE getting into visorinput_open() during the call to input_register_device() that was done before device initialization was complete, which was BEFORE we initialized the semaphore. There is a 2nd follow-on patch that actually does the simple semaphore --> mutex conversion at https://github.com/davidker/unisys/commit/6f57ed62ae206c23c58ce4a016b08e15639ce9af. > > Is that acceptable for you? > > Please fix it before moving the drivers out of staging. Absolutely. We will probably push that patchset (containing the 2 github patches referenced above) within the next few days, even if this visorbus patchset hasn't moved. Thanks. Tim Sell > > Thanks, > > tglx _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
