linux-kernel - Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls for FETCH-OP flavors

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:	Fri, 24 Jun 2016 10:44:04 -0700
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Davidlohr Bueso <dave@...olabs.net>
Cc:	peterz@...radead.org, mingo@...nel.org, davem@...emloft.net,
	cw00.choi@...sung.com, dougthompson@...ssion.com, bp@...en8.de,
	mchehab@....samsung.com, gregkh@...uxfoundation.org, pfg@....com,
	jikos@...nel.org, hans.verkuil@...co.com, awalls@...metrocast.net,
	dledford@...hat.com, sean.hefty@...el.com, kys@...rosoft.com,
	heiko.carstens@...ibm.com, sumit.semwal@...aro.org,
	schwidefsky@...ibm.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH -tip 00/12] locking/atomics: Add and use inc,dec calls
 for FETCH-OP flavors

On Fri, 2016-06-24 at 10:30 -0700, Davidlohr Bueso wrote:
> On Fri, 24 Jun 2016, James Bottomley wrote:
> 
> > On Mon, 2016-06-20 at 13:05 -0700, Davidlohr Bueso wrote:
> > > Hi,
> > > 
> > > The series is really straightforward and based on Peter's work 
> > > that introduces[1] the atomic_fetch_$op machinery. Only patch 1
> > > implements the actual atomic_fetch_{inc,dec} calls based on
> > > atomic_fetch_{add,sub}.
> > 
> > Could I just ask why?  atomic_inc_return(x) - 1 seems a reasonable
> > thing to do to me.
> 
> For one restoring the old state like that can be racy and looses the 
> notion of atomicity.

I don't understand this argument: any return of an atomic value is
inherently racy because the atomic source may have changed by the time
you use the returned value.  It's no more or less racy to my mind to
return the original value and increment than to return the incremented
value and subtract one.

>  The new family of atomic_fetch_$ops also better express the purpose
> of the call imo.

So this is probably the core of my objection: adding APIs simply
because we can.  A good reason to add things like this is because it's
a common pattern people get wrong, because we can optimize it nicely on
an architecture, or some other good reason.  Absent a good reason it
doesn't seem like a good API addition because trying to keep up with
all the API variants when you want to use atomics adds to the burden of
the programmer.

>  Finally, the added machinery (considering it came from fetch_op()
> NOHZ needs), was mainly suggested by Linus (although yes, we don't
> have users for all the calls): 
> https://lkml.org/lkml/2016/3/15/352.

I'd really rather you told me *you* believed it was a good idea (and
why) ...

James