| Message ID | 20200930112612.76109-2-farman@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix slub_debug on 5.9-rc | expand |
On 9/30/20 1:26 PM, Eric Farman wrote: > The routine that applies debug flags to the kmem_cache slabs > inadvertantly prevents non-debug flags from being applied to > those same objects. That is, if slub_debug=<flag>,<slab> is > specified, non-debugged slabs will end up having flags of zero, > and the slabs will be unusable. Fix this by returning the input > flags for non-matching slabs as was done previously. Thanks a lot for debugging this and sorry for the trouble! > Fixes: e17f1dfba37b ("mm, slub: extend slub_debug syntax for multiple blocks") > Signed-off-by: Eric Farman <farman@linux.ibm.com> Acked-by: Vlastimil Babka <vbabka@suse.cz> But with a small adjustment below: > --- > mm/slub.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/slub.c b/mm/slub.c > index d4177aecedf6..3d7c95fd6a08 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1450,7 +1450,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, > } > } > > - return slub_debug; > + return flags; To keep supporting the case of "debug flags set for all caches, with exceptions for listed caches", i.e. "slub_debug=FZ;-,zs_handle,zspage", we should return here this: return flags | slub_debug; Thanks again! > } > #else /* !CONFIG_SLUB_DEBUG */ > static inline void setup_object_debug(struct kmem_cache *s, >
On 9/30/20 7:37 AM, Vlastimil Babka wrote: > On 9/30/20 1:26 PM, Eric Farman wrote: >> The routine that applies debug flags to the kmem_cache slabs >> inadvertantly prevents non-debug flags from being applied to >> those same objects. That is, if slub_debug=<flag>,<slab> is >> specified, non-debugged slabs will end up having flags of zero, >> and the slabs will be unusable. Fix this by returning the input >> flags for non-matching slabs as was done previously. > > Thanks a lot for debugging this and sorry for the trouble! You're welcome. Just glad I wasn't losing my mind! > >> Fixes: e17f1dfba37b ("mm, slub: extend slub_debug syntax for multiple blocks") >> Signed-off-by: Eric Farman <farman@linux.ibm.com> > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > > But with a small adjustment below: > >> --- >> mm/slub.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index d4177aecedf6..3d7c95fd6a08 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -1450,7 +1450,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, >> } >> } >> >> - return slub_debug; >> + return flags; > > To keep supporting the case of "debug flags set for all caches, with exceptions > for listed caches", i.e. "slub_debug=FZ;-,zs_handle,zspage", we should return > here this: > > return flags | slub_debug; Ah, cool... I wondered about that, but didn't go far enough down the combinations. Does it then make sense to strip out the "if (!slub_debug_string)" check at the beginning of the function? As in: ----8<---- diff --git a/mm/slub.c b/mm/slub.c index 3d7c95fd6a08..6d3574013b2f 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1413,10 +1413,6 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, char *next_block; slab_flags_t block_flags; - /* If slub_debug = 0, it folds into the if conditional. */ - if (!slub_debug_string) - return flags | slub_debug; - len = strlen(name); next_block = slub_debug_string; /* Go through all blocks of debug options, see if any matches our slab's name */ @@ -1450,7 +1446,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, } } - return flags; + return flags | slub_debug; } #else /* !CONFIG_SLUB_DEBUG */ static inline void setup_object_debug(struct kmem_cache *s, > > Thanks again! > >> } >> #else /* !CONFIG_SLUB_DEBUG */ >> static inline void setup_object_debug(struct kmem_cache *s, >> >
On 9/30/20 3:06 PM, Eric Farman wrote: > > > On 9/30/20 7:37 AM, Vlastimil Babka wrote: >> On 9/30/20 1:26 PM, Eric Farman wrote: >>> The routine that applies debug flags to the kmem_cache slabs >>> inadvertantly prevents non-debug flags from being applied to >>> those same objects. That is, if slub_debug=<flag>,<slab> is >>> specified, non-debugged slabs will end up having flags of zero, >>> and the slabs will be unusable. Fix this by returning the input >>> flags for non-matching slabs as was done previously. >> >> Thanks a lot for debugging this and sorry for the trouble! > > You're welcome. Just glad I wasn't losing my mind! > >> >>> Fixes: e17f1dfba37b ("mm, slub: extend slub_debug syntax for multiple blocks") >>> Signed-off-by: Eric Farman <farman@linux.ibm.com> >> >> Acked-by: Vlastimil Babka <vbabka@suse.cz> >> >> But with a small adjustment below: >> >>> --- >>> mm/slub.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/mm/slub.c b/mm/slub.c >>> index d4177aecedf6..3d7c95fd6a08 100644 >>> --- a/mm/slub.c >>> +++ b/mm/slub.c >>> @@ -1450,7 +1450,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, >>> } >>> } >>> >>> - return slub_debug; >>> + return flags; >> >> To keep supporting the case of "debug flags set for all caches, with exceptions >> for listed caches", i.e. "slub_debug=FZ;-,zs_handle,zspage", we should return >> here this: >> >> return flags | slub_debug; > > Ah, cool... I wondered about that, but didn't go far enough down the > combinations. Does it then make sense to strip out the "if > (!slub_debug_string)" check at the beginning of the function? As in: Yeah, that makes sense. Thanks!
diff --git a/mm/slub.c b/mm/slub.c index d4177aecedf6..3d7c95fd6a08 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1450,7 +1450,7 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, } } - return slub_debug; + return flags; } #else /* !CONFIG_SLUB_DEBUG */ static inline void setup_object_debug(struct kmem_cache *s,
The routine that applies debug flags to the kmem_cache slabs inadvertantly prevents non-debug flags from being applied to those same objects. That is, if slub_debug=<flag>,<slab> is specified, non-debugged slabs will end up having flags of zero, and the slabs will be unusable. Fix this by returning the input flags for non-matching slabs as was done previously. Fixes: e17f1dfba37b ("mm, slub: extend slub_debug syntax for multiple blocks") Signed-off-by: Eric Farman <farman@linux.ibm.com> --- mm/slub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)