linux-kernel - Re: [PATCH 0/5] xfs refcount conversions

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20171020232111.GT3666@dastard>
Date:   Sat, 21 Oct 2017 10:21:11 +1100
From:   Dave Chinner <david@...morbit.com>
To:     Elena Reshetova <elena.reshetova@...el.com>
Cc:     darrick.wong@...cle.com, linux-kernel@...r.kernel.org,
        linux-xfs@...r.kernel.org, peterz@...radead.org,
        keescook@...omium.org
Subject: Re: [PATCH 0/5] xfs refcount conversions

On Fri, Oct 20, 2017 at 02:07:53PM +0300, Elena Reshetova wrote:
> Note: our previous thread didn't finish in any conclusion, so
> I am resending this now (rebased at latest linux-next) to revive
> the discussion. refcount_t is slowly becoming a standard for
> refcounters and we would really like to make all conversions
> done where it is applicable.

In a separate "replace atomics with refcounts" discussion, the
ordering guarantees of refcounts was raised:

https://lkml.org/lkml/2017/9/4/206

i.e. refcounts use weak ordering whilst atomics imply a smp_mb()
operation was performed.

Given these counters in XFS directly define the life cycle states
rather than being just an object refcount, I'm pretty sure they
rely on the implied smp_mb() that the atomic operations provide to
work correctly.

Let me put it this way: Documentation/memory-barriers.txt breaks my
brain.

We know atomics provide the barriers we need for the code to work
correctly, but using anything else requires understanding the
ordering of the new code and what Documentation/memory-barriers.txt
says that means.

IMO, that makes it way too hard to review sanely for code that:

	a) we already know works correctly
	b) has guards against reference count problems; and
	c) isn't performance critical, so doesn't need the
	   complexity of tricky memory ordering and/or barriers.

So, really, it comes down to the fact that we know refcount_t is not
a straight drop in replacement for atomics, and that actually
verifying the change is correct requires an in depth understanding
of Documentation/memory-barriers.txt. IMO, that's way too much of a
long term maintenance and knowledge burden to add to what is a
simple set of reference counters...

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com