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@xxxxxxxxxxxxx>
>
> Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
>
> 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,
>>
>
