Re: [PATCH v2] mm/slub.c: replace kmem_cache->cpu_partial with wrapped APIs

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 26 Feb 2020 19:13:31 +0100 Vlastimil Babka <vbabka@suse.cz> wrote:

> On 2/22/20 4:40 AM, Christopher Lameter wrote:
> > On Wed, 19 Feb 2020, qiwuchen55@gmail.com wrote:
> > 
> >> diff --git a/mm/slub.c b/mm/slub.c
> >> index 17dc00e..1eb888c 100644
> >> --- a/mm/slub.c
> >> +++ b/mm/slub.c
> >> @@ -2284,7 +2284,7 @@ static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain)
> >>  		if (oldpage) {
> >>  			pobjects = oldpage->pobjects;
> >>  			pages = oldpage->pages;
> >> -			if (drain && pobjects > s->cpu_partial) {
> >> +			if (drain && pobjects > slub_cpu_partial(s)) {
> >>  				unsigned long flags;
> >>  				/*
> >>  				 * partial array is full. Move the existing
> > 
> > Maybe its better to not generate code for put_cpu_partial() instead of
> > using macros there if per cpu partials are disabled?
> 
> The whole code of put_cpu_partial() is already under
> #ifdef CONFIG_SLUB_CPU_PARTIAL.
> 
> I agree that the wrapper shouldn't be used in a function that deals only with
> the case that the partials do exist. It just obscures the code unnecessarily.

Yes, I scratched my head over this and decided to go ahead with the
patches.  Given that we have an accessor for ->cpu_partial, it's best
that it be used consistently.  The fact that the wrapper is there to
avoid ifdefs is an implementation detail which is private to the header
files and Kconfig system.

IOW, the way we open-code it in some places and use the accessor in
other places also obscures the code.  Which is preferable?  I don't see
a clear answer, but lean towards consistency.