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:   Mon, 11 Oct 2021 07:23:35 +0000
From:   Hyeonggon Yoo <42.hyeyoo@gmail.com>
To:     David Rientjes <rientjes@google.com>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH] mm, slub: Use prefetchw instead of prefetch
Message-ID: <20211011072335.GA63489@kvm.asia-northeast3-a.c.our-ratio-313919.internal>
References: <20211008133602.4963-1-42.hyeyoo@gmail.com>
 <30a76d87-e0af-3eec-d095-d87e898b31cf@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <30a76d87-e0af-3eec-d095-d87e898b31cf@google.com>
Precedence: bulk

On Sun, Oct 10, 2021 at 03:49:07PM -0700, David Rientjes wrote:
> On Fri, 8 Oct 2021, Hyeonggon Yoo wrote:
> 
> > It's certain that an object will be not only read, but also
> > written after allocation.
> > 
> 
> Why is it certain?  I think perhaps what you meant to say is that if we 
> are doing any prefetching here, then access will benefit from prefetchw 
> instead of prefetch.  But it's not "certain" that allocated memory will be 
> accessed at all.
>

Blame my english skill :(

When I wrote I thought it was ok, but it was unclear.
Thank you for pointing them!

What I meant was "When accessing an object, it must be written before read.
So There's no situation that caller only reads an object and does not
write. Thus it's better to use prefetchw instead of prefetch.".

Let's rephrase:

commit 0ad9500e16fe ("slub: prefetch next freelist pointer in 
slab_alloc()") introduced prefetch_freepointer() because when other cpu(s)
freed objects into a page that current cpu owns, the freelist link is
hot on cpu(s) which freed objects and possibly very cold on current cpu.

But if freelist link chain is hot on cpu(s) which freed	objects,
it's better to invalidate that chain because they're not going to access
again within a short time.

So use prefetchw instead of prefetch. On supported architectures like x86,
it invalidates other copied instances of a cache line when prefetching it.

> > Use prefetchw instead of prefetchw. On supported architecture
> 
> If we're using prefetchw instead of prefetchw, I think the diff would be 
> 0 lines changed :)
>

That was my typo. thankfully Andrew fixed that.

> > like x86, it helps to invalidate cache line when the object exists
> > in other processors' cache.
> > 
> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> > ---
> >  mm/slub.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 3d2025f7163b..2aca7523165e 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -352,9 +352,9 @@ static inline void *get_freepointer(struct kmem_cache *s, void *object)
> >  	return freelist_dereference(s, object + s->offset);
> >  }
> >  
> > -static void prefetch_freepointer(const struct kmem_cache *s, void *object)
> > +static void prefetchw_freepointer(const struct kmem_cache *s, void *object)
> >  {
> > -	prefetch(object + s->offset);
> > +	prefetchw(object + s->offset);
> >  }
> >  
> >  static inline void *get_freepointer_safe(struct kmem_cache *s, void *object)
> > @@ -3195,10 +3195,9 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
> >  			note_cmpxchg_failure("slab_alloc", s, tid);
> >  			goto redo;
> >  		}
> > -		prefetch_freepointer(s, next_object);
> > +		prefetchw_freepointer(s, next_object);
> >  		stat(s, ALLOC_FASTPATH);
> >  	}
> > -
> >  	maybe_wipe_obj_freeptr(s, object);
> >  	init = slab_want_init_on_alloc(gfpflags, s);
> >  
> > -- 
> > 2.27.0
> > 
> >