RE: [PATCH v4 09/29] staging: unisys: visorinput: remove unnecessary locking

[Sell, Timothy C]

> > > > 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/039e6e517b4a17e2d135a9df85
> cc1e24a39c2670.
> 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/6f57ed62ae206c23c58ce4a016
> b08e15639ce9af.
> 
> > > 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.
> 

The 2 patches referenced above (which include the bug fix, and the change
from semaphore to mutex) are included as patches #27 and #28 in the
patchset just submitted by David Kershner:

	[PATCH RESEND 00/28] staging: unisys: fix visorbus & visorinput issues raised by tglx

Thanks.

Tim Sell