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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx]
> Sent: Thursday, June 09, 2016 5:01 AM
> To: Kershner, David A
> Cc: corbet@xxxxxxx; mingo@xxxxxxxxxx; hpa@xxxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; Arfvidson, Erik; Sell, Timothy C;
> 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
> Subject: Re: [PATCH v4 09/29] staging: unisys: visorinput: remove
> unnecessary locking
> 
> On Wed, 8 Jun 2016, David Kershner wrote:
> > +	/*
> > +	 * If we're not paused, really enable interrupts.
> > +	 * Regardless of whether we are paused, set a flag indicating
> > +	 * interrupts should be enabled so when we resume, interrupts
> > +	 * will really be enabled.
> > +	 */
> > +	down_write(&devdata->lock_visor_dev);
> 
> 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?
> 
> Thanks,
> 
> 	tglx

(Yes, I attempted to address this question in a post I made
6/3/2016 4:30 UTC, which I have basically pasted below.)

You are correct that this should be a mutex.

We have a local patch that addresses this, but would like
to submit this via a follow-on patchset if possible.  I'll explain.

Rationale: our intent for this patchset was to focus on the visorbus
driver ONLY.  The only reason visorinput got involved in the first place
was due to the visorbus change that necessitated that we remove the locking
from visorinput_channel_interrupt(), due to that now being called from atomic
context.

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.

Is that acceptable for you?

Tim Sell

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux