Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 3 Mar 2021 19:30:38 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Christoph Lameter <cl@gentwo.de>
Cc:     Xunlei Pang <xlpang@linux.alibaba.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Wen Yang <wenyang@linux.alibaba.com>,
        Roman Gushchin <guro@fb.com>, Pekka Enberg <penberg@gmail.com>,
        Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
        David Rientjes <rientjes@google.com>,
        linux-kernel@vger.kernel.org,
        "linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH v2 3/3] mm/slub: Use percpu partial free counter
Message-ID: <20210303193038.GE2723601@casper.infradead.org>
References: <1597061872-58724-1-git-send-email-xlpang@linux.alibaba.com>
 <1597061872-58724-4-git-send-email-xlpang@linux.alibaba.com>
 <alpine.DEB.2.22.394.2103021012010.860725@gentwo.de>
 <20210303142612.GC2723601@casper.infradead.org>
 <alpine.DEB.2.22.394.2103032012250.896915@gentwo.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <alpine.DEB.2.22.394.2103032012250.896915@gentwo.de>
Precedence: bulk

On Wed, Mar 03, 2021 at 08:15:58PM +0100, Christoph Lameter wrote:
> On Wed, 3 Mar 2021, Matthew Wilcox wrote:
> 
> > On Tue, Mar 02, 2021 at 10:14:53AM +0100, Christoph Lameter wrote:
> > > On Mon, 10 Aug 2020, Xunlei Pang wrote:
> > > > -	atomic_long_t partial_free_objs;
> > > > +	atomic_long_t __percpu *partial_free_objs;
> > >
> > > A percpu counter is never atomic. Just use unsigned long and use this_cpu
> > > operations for this thing. That should cut down further on the overhead.
> >
> > What about allocations from interrupt context?  Should this be a local_t
> > instead?
> 
> Can this be allocated in an interrupt context?
> 
> And I am not sure how local_t relates to that? Percpu counters can be used
> in an interrupt context without the overhead of the address calculations
> that are required by a local_t.

As I understand the patch, this counts the number of partially free slabs.
So if we start to free an object from a completely full slab in process
context, as "load x, add one to x, store x" and take an interrupt
between loading x and adding one to x, that interrupt handler might
free a different object from another completely full slab.  that would
also load the same x, add one to it and store x, but then the process
context would add one to the old x, overwriting the updated value from
interrupt context.

it's not the likeliest of races, and i don't know how important it is
that these counters remain accurate.  but using a local_t instead of
a percpu long would fix the problem.  i don't know why you think that
a local_t needs "address calculations".  perhaps you've misremembered
what a local_t is?