From: xunlei <xlpang@linux.alibaba.com>
To: Pekka Enberg <penberg@gmail.com>, Christopher Lameter <cl@linux.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
Andrew Morton <akpm@linux-foundation.org>,
Wen Yang <wenyang@linux.alibaba.com>,
Yang Shi <yang.shi@linux.alibaba.com>,
Roman Gushchin <guro@fb.com>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
LKML <linux-kernel@vger.kernel.org>,
Konstantin Khlebnikov <khlebnikov@yandex-team.ru>,
David Rientjes <rientjes@google.com>
Subject: Re: [PATCH 1/2] mm/slub: Introduce two counters for the partial objects
Date: Mon, 24 Aug 2020 17:59:46 +0800 [thread overview]
Message-ID: <9811b473-e09f-c2aa-cdd8-c71c34fe4707@linux.alibaba.com> (raw)
In-Reply-To: <CAOJsxLHnTDjY3X69zVVaZ0MbpRab-1+Rk3L5AnZD3oQEceHD7Q@mail.gmail.com>
On 2020/8/20 PM9:58, Pekka Enberg wrote:
> Hi Christopher,
>
> On Tue, Aug 11, 2020 at 3:52 PM Christopher Lameter <cl@linux.com> wrote:
>>
>> On Fri, 7 Aug 2020, Pekka Enberg wrote:
>>
>>> Why do you consider this to be a fast path? This is all partial list
>>> accounting when we allocate/deallocate a slab, no? Just like
>>> ___slab_alloc() says, I assumed this to be the slow path... What am I
>>> missing?
>>
>> I thought these were per object counters? If you just want to count the
>> number of slabs then you do not need the lock at all. We already have a
>> counter for the number of slabs.
>
> The patch attempts to speed up count_partial(), which holds on to the
> "n->list_lock" (with IRQs off) for the whole duration it takes to walk
> the partial slab list:
>
> spin_lock_irqsave(&n->list_lock, flags);
> list_for_each_entry(page, &n->partial, slab_list)
> x += get_count(page);
> spin_unlock_irqrestore(&n->list_lock, flags);
>
> It's counting the number of *objects*, but the counters are only
> updated in bulk when we add/remove a slab to/from the partial list.
> The counter updates are therefore *not* in the fast-path AFAICT.
>
> Xunlei, please correct me if I'm reading your patches wrong.
Yes, it's all in slow-path.
>
> On Tue, Aug 11, 2020 at 3:52 PM Christopher Lameter <cl@linux.com> wrote:
>>> No objections to alternative fixes, of course, but wrapping the
>>> counters under CONFIG_DEBUG seems like just hiding the actual issue...
>>
>> CONFIG_DEBUG is on by default. It just compiles in the debug code and
>> disables it so we can enable it with a kernel boot option. This is because
>> we have had numerous issues in the past with "production" kernels that
>> could not be recompiled with debug options. So just running the prod
>> kernel with another option will allow you to find hard to debug issues in
>> a full scale producton deployment with potentially proprietary modules
>> etc.
>
> Yeah, it's been too long since I last looked at the code and did not
> realize even count_partial() is wrapped in CONFIG_DEBUG. So by all
Besides CONFIG_DEBUG, count_partial() is also wrapped in CONFIG_SYSFS.
> means, let's also wrap the counters with that too.
>
> - Pekka
>
prev parent reply other threads:[~2020-08-24 9:59 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-02 8:32 [PATCH 1/2] mm/slub: Introduce two counters for the partial objects Xunlei Pang
2020-07-02 8:32 ` [PATCH 2/2] mm/slub: Get rid of count_partial() Xunlei Pang
2020-07-02 11:59 ` [PATCH 1/2] mm/slub: Introduce two counters for the partial objects Pekka Enberg
2020-07-03 9:37 ` xunlei
2020-07-07 15:23 ` Pekka Enberg
2020-07-09 14:32 ` Christopher Lameter
2020-07-31 2:57 ` xunlei
2020-07-07 6:59 ` Christopher Lameter
2020-07-31 2:52 ` xunlei
2020-08-06 12:42 ` Vlastimil Babka
2020-08-07 7:25 ` Pekka Enberg
2020-08-07 13:02 ` Christopher Lameter
2020-08-07 17:28 ` Pekka Enberg
2020-08-10 11:56 ` xunlei
2020-08-11 12:52 ` Christopher Lameter
2020-08-20 13:58 ` Pekka Enberg
2020-08-24 9:59 ` xunlei [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9811b473-e09f-c2aa-cdd8-c71c34fe4707@linux.alibaba.com \
--to=xlpang@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=guro@fb.com \
--cc=khlebnikov@yandex-team.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=penberg@gmail.com \
--cc=rientjes@google.com \
--cc=vbabka@suse.cz \
--cc=wenyang@linux.alibaba.com \
--cc=yang.shi@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).