[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20181115122444.GA29281@Red>
Date: Thu, 15 Nov 2018 13:24:44 +0100
From: LABBE Corentin <clabbe@...libre.com>
To: Russell King - ARM Linux <linux@...linux.org.uk>
Cc: Gilles.Muller@...6.fr, Julia.Lawall@...6.fr, agust@...x.de,
airlied@...ux.ie, alexandre.torgue@...com, alistair@...ple.id.au,
benh@...nel.crashing.org, carlo@...one.org, davem@...emloft.net,
galak@...nel.crashing.org, joabreu@...opsys.com,
khilman@...libre.com, matthias.bgg@...il.com,
maxime.ripard@...tlin.com, michal.lkml@...kovi.net,
mpe@...erman.id.au, mporter@...nel.crashing.org,
narmstrong@...libre.com, nicolas.palix@...g.fr, oss@...error.net,
paulus@...ba.org, peppe.cavallaro@...com, tj@...nel.org,
vitb@...nel.crashing.org, wens@...e.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
linux-ide@...r.kernel.org, linux-sunxi@...glegroups.com,
linux-mediatek@...ts.infradead.org,
linux-amlogic@...ts.infradead.org, linuxppc-dev@...ts.ozlabs.org,
cocci@...teme.lip6.fr, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 0/7] include: add
setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64
On Thu, Nov 15, 2018 at 09:33:48AM +0000, Russell King - ARM Linux wrote:
> On Thu, Nov 15, 2018 at 10:30:34AM +0100, LABBE Corentin wrote:
> > On Wed, Oct 24, 2018 at 09:57:00AM +0100, Russell King - ARM Linux wrote:
> > > On Wed, Oct 24, 2018 at 07:35:46AM +0000, Corentin Labbe wrote:
> > > > This patchset adds a new set of functions which are open-coded in lot of
> > > > place.
> > > > Basicly the pattern is always the same, "read, modify a bit, write"
> > > > some driver and the powerpc arch already have thoses pattern them as functions. (like ahci_sunxi.c or dwmac-meson8b)
> > >
> > > The advantage of them being open-coded is that it's _obvious_ to the
> > > reviewer that there is a read-modify-write going on which, in a multi-
> > > threaded environment, may need some locking (so it should trigger a
> > > review of the locking around that code.)
> > >
> > > With it hidden inside a helper which has no locking itself, it becomes
> > > much easier to pass over in review, which means that races are much
> > > more likely to go unspotted - and that is bad news.
> > >
> >
> > Hello
> >
> > I understand your fear, but I think the benefit overhaul thoses.
> > Furthermore, drivers which I have converted does not need such locking.
> >
> > If you want I can rename the header to linux/setbits-non-atomic.h for making obvious the lack of locking.
>
> It'd probably be better in the function name - it then doesn't get
> "lost" that it's non-atomic when it's included via other headers.
>
I proposed that way for doing like writeq have do it with io-64-nonatomic-hi-lo.h
Powered by blists - more mailing lists