[PATCH] slab: Clean up the code comment in slab kmem_cache struct

[PATCH] slab: Clean up the code comment in slab kmem_cache struct

[Baoquan He]

In commit

  3b0efdfa1e7("mm, sl[aou]b: Extract common fields from struct kmem_cache")

The variable 'obj_size' was moved above, however the related code comment
is not updated accordingly. Do it here.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 include/linux/slab_def.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
index d9228e4d0320..3485c58cfd1c 100644
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -67,9 +67,10 @@ struct kmem_cache {
 
 	/*
 	 * If debugging is enabled, then the allocator can add additional
-	 * fields and/or padding to every object. size contains the total
-	 * object size including these internal fields, the following two
-	 * variables contain the offset to the user object and its size.
+	 * fields and/or padding to every object. 'size' contains the total
+	 * object size including these internal fields, while 'obj_offset'
+	 * and 'object_size' contain the offset to the user object and its
+	 * size.
 	 */
 	int obj_offset;
 #endif /* CONFIG_DEBUG_SLAB */
-- 
2.13.6

Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct

[Christopher Lameter]

On Sun, 3 Jun 2018, Baoquan He wrote:

> diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> index d9228e4d0320..3485c58cfd1c 100644
> --- a/include/linux/slab_def.h
> +++ b/include/linux/slab_def.h
> @@ -67,9 +67,10 @@ struct kmem_cache {
>
>  	/*
>  	 * If debugging is enabled, then the allocator can add additional
> -	 * fields and/or padding to every object. size contains the total
> -	 * object size including these internal fields, the following two
> -	 * variables contain the offset to the user object and its size.
> +	 * fields and/or padding to every object. 'size' contains the total
> +	 * object size including these internal fields, while 'obj_offset'
> +	 * and 'object_size' contain the offset to the user object and its
> +	 * size.
>  	 */
>  	int obj_offset;
>  #endif /* CONFIG_DEBUG_SLAB */
>

Wish we had some more consistent naming. object_size and obj_offset??? And
the fields better be as close together as possible.


Acked-by: Christoph Lameter <cl@linux.com>

Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct

[Baoquan He]

On 06/05/18 at 05:04pm, Christopher Lameter wrote:
> On Sun, 3 Jun 2018, Baoquan He wrote:
> 
> > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> > index d9228e4d0320..3485c58cfd1c 100644
> > --- a/include/linux/slab_def.h
> > +++ b/include/linux/slab_def.h
> > @@ -67,9 +67,10 @@ struct kmem_cache {
> >
> >  	/*
> >  	 * If debugging is enabled, then the allocator can add additional
> > -	 * fields and/or padding to every object. size contains the total
> > -	 * object size including these internal fields, the following two
> > -	 * variables contain the offset to the user object and its size.
> > +	 * fields and/or padding to every object. 'size' contains the total
> > +	 * object size including these internal fields, while 'obj_offset'
> > +	 * and 'object_size' contain the offset to the user object and its
> > +	 * size.
> >  	 */
> >  	int obj_offset;
> >  #endif /* CONFIG_DEBUG_SLAB */
> >
> 
> Wish we had some more consistent naming. object_size and obj_offset??? And
> the fields better be as close together as possible.

I am back porting Thomas's sl[a|u]b freelist randomization feature to
our distros, need go through slab code for better understanding. From
git log history, they were 'obj_offset' and 'obj_size'. Later on
'obj_size' was renamed to 'object_size' in commit 3b0efdfa1e("mm, sl[aou]b:
Extract common fields from struct kmem_cache") which is from your patch.
With my understanding, I guess you changed that on purpose because
object_size is size of each object, obj_offset is for the whole cache,
representing the offset the real object starts to be stored. And putting
them separately is for better desribing them in code comment and
distinction, e.g 'object_size' is in "4) cache creation/removal",
while 'obj_offset' is put alone to indicate it's for the whole.

Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct

[Christopher Lameter]

On Wed, 6 Jun 2018, Baoquan He wrote:

> I am back porting Thomas's sl[a|u]b freelist randomization feature to
> our distros, need go through slab code for better understanding. From
> git log history, they were 'obj_offset' and 'obj_size'. Later on
> 'obj_size' was renamed to 'object_size' in commit 3b0efdfa1e("mm, sl[aou]b:
> Extract common fields from struct kmem_cache") which is from your patch.
> With my understanding, I guess you changed that on purpose because
> object_size is size of each object, obj_offset is for the whole cache,
> representing the offset the real object starts to be stored. And putting
> them separately is for better desribing them in code comment and
> distinction, e.g 'object_size' is in "4) cache creation/removal",
> while 'obj_offset' is put alone to indicate it's for the whole.

obj_offset only applies when CONFIG_SLAB_DEBUG is set. Ok so that screwy
name also indicates that something special goes on.

Re: [PATCH] slab: Clean up the code comment in slab kmem_cache struct

[Baoquan He]

On 06/06/18 at 11:48pm, Christopher Lameter wrote:
> On Wed, 6 Jun 2018, Baoquan He wrote:
> 
> > I am back porting Thomas's sl[a|u]b freelist randomization feature to
> > our distros, need go through slab code for better understanding. From
> > git log history, they were 'obj_offset' and 'obj_size'. Later on
> > 'obj_size' was renamed to 'object_size' in commit 3b0efdfa1e("mm, sl[aou]b:
> > Extract common fields from struct kmem_cache") which is from your patch.
> > With my understanding, I guess you changed that on purpose because
> > object_size is size of each object, obj_offset is for the whole cache,
> > representing the offset the real object starts to be stored. And putting
> > them separately is for better desribing them in code comment and
> > distinction, e.g 'object_size' is in "4) cache creation/removal",
> > while 'obj_offset' is put alone to indicate it's for the whole.
> 
> obj_offset only applies when CONFIG_SLAB_DEBUG is set. Ok so that screwy
> name also indicates that something special goes on.

They are a little confusing when combine SLAB with SLUB, 

SLAB			SLUB
size                    size
object_size             object_size
obj_offset              offset

object_size also only applies when CONFIG_SLAB_DEBUG is set,
otherwise it's equal to size in SLAB case. Whereas SLUB always has
object plus freeptr for each object space.

Thought to add code comment to make them clearer, on second thought,
anyone who want to understand slab/slub code well, has to go through the
whole header file and souce code, this pain need be borne.