linux-kernel - Re: [RFC][PATCH] mfd: mc13xxx-core: put mutex lock down to mc13xxx_reg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 3 May 2011 22:41:55 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
Cc:	Axel Lin <axel.lin@...il.com>, linux-kernel@...r.kernel.org,
	Yong Shen <yong.shen@...aro.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Liam Girdwood <lrg@...mlogic.co.uk>
Subject: Re: [RFC][PATCH] mfd: mc13xxx-core: put mutex lock down to
 mc13xxx_reg_rmw function

On Tue, May 03, 2011 at 09:15:25PM +0200, Uwe Kleine-König wrote:
> On Wed, May 04, 2011 at 12:27:59AM +0800, Axel Lin wrote:

> > +	ret = mc13xxx_reg_rmw(priv->mc13xxx, mc13892_regulators[id].vsel_reg,
> > +			mask, value << mc13892_regulators[id].vsel_shift);

> I havn't looked deeply, but I guess this can have unwanted side effects
> here. Before you had:

> 	lock()
> 	do(something)
> 	do(something, else, that, needs, rmw)
> 	unlock()

> and you introduced an unlock()/lock() between these two do()s.

Glancing at the code I wasn't 100% convinced that the original read was
really needed, though I didn't look closely.

> I'm not convinced this change is good, though I agree that

> 	lock()
> 	rmw(...)
> 	unlock()

> looks ugly, but imho this can better be fixed by adding a wrapper for
> that sequence if you really want.

You could also make the rmw store the value somewhere if it's important.
Having to open code the locks everywhere is certainly annoying and error
prone.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/