Re: [PATCH 4.14] mm, slub: restore the original intention of prefetch_freepointer()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/26/20 9:06 AM, Sven Eckelmann wrote:
From: Vlastimil Babka <vbabka@xxxxxxx>

commit 0882ff9190e3bc51e2d78c3aadd7c690eeaa91d5 upstream.

In SLUB, prefetch_freepointer() is used when allocating an object from
cache's freelist, to make sure the next object in the list is cache-hot,
since it's probable it will be allocated soon.

Commit 2482ddec670f ("mm: add SLUB free list pointer obfuscation") has
unintentionally changed the prefetch in a way where the prefetch is
turned to a real fetch, and only the next->next pointer is prefetched.
In case there is not a stream of allocations that would benefit from
prefetching, the extra real fetch might add a useless cache miss to the
allocation.  Restore the previous behavior.

Link: http://lkml.kernel.org/r/20180809085245.22448-1-vbabka@xxxxxxx
Fixes: 2482ddec670f ("mm: add SLUB free list pointer obfuscation")
Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Daniel Micay <danielmicay@xxxxxxxxx>
Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
Cc: Christoph Lameter <cl@xxxxxxxxx>
Cc: Pekka Enberg <penberg@xxxxxxxxxx>
Cc: David Rientjes <rientjes@xxxxxxxxxx>
Cc: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
Cc: Matthias Schiffer <mschiffer@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Sven Eckelmann <sven@xxxxxxxxxxxxx>

Regardless of how the discussion of exact root cause turns out, I agree with the stable inclusion, consider this as Ack, if it makes sense in that context :)

---
The original problem is explained in the patch description as
performance problem. And maybe this could also be one reason why it was
never submitted for a stable kernel.

But tests on mips ath79 (OpenWrt ar71xx target) showed that it most likely
related to "random" data bus errors. At least applying this patch seemed to
have solved it for Matthias Schiffer <mschiffer@xxxxxxxxxxxxxxxxxxxx> and
some other persons who where debugging/testing this problem with him.

More details about it can be found in
https://github.com/freifunk-gluon/gluon/issues/1982
---
  mm/slub.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 3c1a16f03b2b..481518c3f61a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -269,8 +269,7 @@ static inline void *get_freepointer(struct kmem_cache *s, void *object)
static void prefetch_freepointer(const struct kmem_cache *s, void *object)
  {
-	if (object)
-		prefetch(freelist_dereference(s, object + s->offset));
+	prefetch(object + s->offset);
  }
static inline void *get_freepointer_safe(struct kmem_cache *s, void *object)





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux