Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Reshetova, Elena" <elena.reshetova@intel.com>
To:     Dave Chinner <david@fromorbit.com>
CC:     "darrick.wong@oracle.com" <darrick.wong@oracle.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "keescook@chromium.org" <keescook@chromium.org>
Subject: RE: [PATCH 0/5] xfs refcount conversions
Thread-Topic: [PATCH 0/5] xfs refcount conversions
Thread-Index: AQHTSZTUz9fYd2XtRkWKd616yhL5h6LtUE2AgAPuSiA=
Date:   Mon, 23 Oct 2017 10:29:51 +0000
Message-ID: <2236FBA76BA1254E88B949DDB74E612B802B436F@IRSMSX102.ger.corp.intel.com>
References: <1508497678-10508-1-git-send-email-elena.reshetova@intel.com>
 <20171020232111.GT3666@dastard>
In-Reply-To: <20171020232111.GT3666@dastard>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> 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...

Fair point. Let's see if we can change refcount_t to provide the same memory ordering
guarantees as atomics. Then you don't have to worry about things like this. 

I think this initial idea behind refcount_t was a straight replacement for atomic_t
reference counter use case plus a number of clear and understandable security guarantees
(like overflow protection).
It was indeed not supposed to make a difference in any *other* ways (and I think in most of cases
it doesn't even in the current form). But it is indeed easier to just make it provide same guarantees 
on ordering vs. trying to check every case (and make sure future users understand this too).

Let me be back that we have a joint conclusion on what to do with memory ordering. 

Best Regards,
Elena.

> 
> Cheers,
> 
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

From 1581820705243234799@xxx Fri Oct 20 23:22:59 +0000 2017
X-GM-THRID: 1581775102950037022
X-Gmail-Labels: Inbox,Category Forums