[RFC] SLUB - define OO_ macro instead of hardcoded numbers

[RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

---

Please check -- wouldn't it be better to use such a macro?
At least it makes easy to understand the 65535 constant is
coming from if only I didn't miss anything :)

 mm/slub.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-2.6.git/mm/slub.c
===================================================================
--- linux-2.6.git.orig/mm/slub.c	2008-09-26 13:45:16.000000000 +0400
+++ linux-2.6.git/mm/slub.c	2008-10-22 20:10:59.000000000 +0400
@@ -153,6 +153,10 @@
 #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif
 
+#define OO_SHIFT	16
+#define OO_MASK		((1 << OO_SHIFT) - 1)
+#define OO_MAX		OO_MASK
+
 /* Internal SLUB flags */
 #define __OBJECT_POISON		0x80000000 /* Poison object */
 #define __SYSFS_ADD_DEFERRED	0x40000000 /* Not yet visible via sysfs */
@@ -290,7 +294,7 @@ static inline struct kmem_cache_order_ob
 						unsigned long size)
 {
 	struct kmem_cache_order_objects x = {
-		(order << 16) + (PAGE_SIZE << order) / size
+		(order << OO_SHIFT) + (PAGE_SIZE << order) / size
 	};
 
 	return x;
@@ -298,12 +302,12 @@ static inline struct kmem_cache_order_ob
 
 static inline int oo_order(struct kmem_cache_order_objects x)
 {
-	return x.x >> 16;
+	return x.x >> OO_SHIFT;
 }
 
 static inline int oo_objects(struct kmem_cache_order_objects x)
 {
-	return x.x & ((1 << 16) - 1);
+	return x.x & OO_MASK;
 }
 
 #ifdef CONFIG_SLUB_DEBUG
@@ -764,8 +768,8 @@ static int on_freelist(struct kmem_cache
 	}
 
 	max_objects = (PAGE_SIZE << compound_order(page)) / s->size;
-	if (max_objects > 65535)
-		max_objects = 65535;
+	if (max_objects > OO_MAX)
+		max_objects = OO_MAX;
 
 	if (page->objects != max_objects) {
 		slab_err(s, page, "Wrong number of objects. Found %d but "
@@ -1819,8 +1823,8 @@ static inline int slab_order(int size, i
 	int rem;
 	int min_order = slub_min_order;
 
-	if ((PAGE_SIZE << min_order) / size > 65535)
-		return get_order(size * 65535) - 1;
+	if ((PAGE_SIZE << min_order) / size > OO_MAX)
+		return get_order(size * OO_MAX) - 1;
 
 	for (order = max(min_order,
 				fls(min_objects * size - 1) - PAGE_SHIFT);

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> Please check -- wouldn't it be better to use such a macro?

Looks good. But could you rename OO_MAX to something different? There is 
already s->max which may cause confusion because s->max is the maximum 
number of objects in a slab. OO_MAX is the maximum mask?

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 09:28:14AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> Please check -- wouldn't it be better to use such a macro?
>
> Looks good. But could you rename OO_MAX to something different? There is  
> already s->max which may cause confusion because s->max is the maximum  
> number of objects in a slab. OO_MAX is the maximum mask?
>

I supposed it would mean maximum object number inside page (ie quantity) which
is happen to be the same value as OO_MASK. Maybe OO_MAX_OBJ?

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Cyrill Gorcunov - Wed, Oct 22, 2008 at 08:35:30PM +0400]
| [Christoph Lameter - Wed, Oct 22, 2008 at 09:28:14AM -0700]
| > On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
| >
| >> Please check -- wouldn't it be better to use such a macro?
| >
| > Looks good. But could you rename OO_MAX to something different? There is  
| > already s->max which may cause confusion because s->max is the maximum  
| > number of objects in a slab. OO_MAX is the maximum mask?
| >
| 
| I supposed it would mean maximum object number inside page (ie quantity) which
| is happen to be the same value as OO_MASK. Maybe OO_MAX_OBJ?
| 
| 		- Cyrill -

Btw Christoph fix me if I'm wrong but this 65535 is directly related to
16 bit shift. If we change the first value without changing the second we
just break the SLUB I guess. I didn't read/understand SLUB code in details
so could be wrong.

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Cyrill Gorcunov - Wed, Oct 22, 2008 at 08:53:54PM +0400]
| [Cyrill Gorcunov - Wed, Oct 22, 2008 at 08:35:30PM +0400]
| | [Christoph Lameter - Wed, Oct 22, 2008 at 09:28:14AM -0700]
| | > On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
| | >
| | >> Please check -- wouldn't it be better to use such a macro?
| | >
| | > Looks good. But could you rename OO_MAX to something different? There is  
| | > already s->max which may cause confusion because s->max is the maximum  
| | > number of objects in a slab. OO_MAX is the maximum mask?
| | >
| | 
| | I supposed it would mean maximum object number inside page (ie quantity) which
| | is happen to be the same value as OO_MASK. Maybe OO_MAX_OBJ?
| | 
| | 		- Cyrill -
| 
| Btw Christoph fix me if I'm wrong but this 65535 is directly related to
| 16 bit shift. If we change the first value without changing the second we
| just break the SLUB I guess. I didn't read/understand SLUB code in details
| so could be wrong.
| 
| 		- Cyrill -

Christoph how about this one?

		- Cyrill -
---

 mm/slub.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-2.6.git/mm/slub.c
===================================================================
--- linux-2.6.git.orig/mm/slub.c	2008-10-22 21:11:26.000000000 +0400
+++ linux-2.6.git/mm/slub.c	2008-10-22 21:19:12.000000000 +0400
@@ -153,6 +153,10 @@
 #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif
 
+#define OO_SHIFT	16
+#define OO_MASK		((1 << OO_SHIFT) - 1)
+#define OO_MAX_OBJS	65535 /* see struct page.objects */
+
 /* Internal SLUB flags */
 #define __OBJECT_POISON		0x80000000 /* Poison object */
 #define __SYSFS_ADD_DEFERRED	0x40000000 /* Not yet visible via sysfs */
@@ -290,7 +294,7 @@ static inline struct kmem_cache_order_ob
 						unsigned long size)
 {
 	struct kmem_cache_order_objects x = {
-		(order << 16) + (PAGE_SIZE << order) / size
+		(order << OO_SHIFT) + (PAGE_SIZE << order) / size
 	};
 
 	return x;
@@ -298,12 +302,12 @@ static inline struct kmem_cache_order_ob
 
 static inline int oo_order(struct kmem_cache_order_objects x)
 {
-	return x.x >> 16;
+	return x.x >> OO_SHIFT;
 }
 
 static inline int oo_objects(struct kmem_cache_order_objects x)
 {
-	return x.x & ((1 << 16) - 1);
+	return x.x & OO_MASK;
 }
 
 #ifdef CONFIG_SLUB_DEBUG
@@ -764,8 +768,8 @@ static int on_freelist(struct kmem_cache
 	}
 
 	max_objects = (PAGE_SIZE << compound_order(page)) / s->size;
-	if (max_objects > 65535)
-		max_objects = 65535;
+	if (max_objects > OO_MAX_OBJS)
+		max_objects = OO_MAX_OBJS;
 
 	if (page->objects != max_objects) {
 		slab_err(s, page, "Wrong number of objects. Found %d but "
@@ -1819,8 +1823,8 @@ static inline int slab_order(int size, i
 	int rem;
 	int min_order = slub_min_order;
 
-	if ((PAGE_SIZE << min_order) / size > 65535)
-		return get_order(size * 65535) - 1;
+	if ((PAGE_SIZE << min_order) / size > OO_MAX_OBJS)
+		return get_order(size * OO_MAX_OBJS) - 1;
 
 	for (order = max(min_order,
 				fls(min_objects * size - 1) - PAGE_SHIFT);

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> Christoph how about this one?

Ok. Looks a bit better but we still have two maxes here

s->max which refers to the maximum number of objects per slab page for 
a specific slab cache (depends on the runtime configuration). OO_MAX_OBJS 
refers to the maximum number of objects per slab page that any slab cache 
can be configured for which is a compile time limit.

Maybe this is okay, Pekka?

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Pekka Enberg]

Christoph Lameter wrote:
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
> 
>> Christoph how about this one?
> 
> Ok. Looks a bit better but we still have two maxes here
> 
> s->max which refers to the maximum number of objects per slab page for a 
> specific slab cache (depends on the runtime configuration). OO_MAX_OBJS 
> refers to the maximum number of objects per slab page that any slab 
> cache can be configured for which is a compile time limit.
> 
> Maybe this is okay, Pekka?

Maybe call the page->objects maximum MAX_OBJS_PER_PAGE as it's not 
strictly related to the other OO code?

		Pekka

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 10:47:09AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> Christoph how about this one?
>
> Ok. Looks a bit better but we still have two maxes here
>
> s->max which refers to the maximum number of objects per slab page for a 
> specific slab cache (depends on the runtime configuration). OO_MAX_OBJS  
> refers to the maximum number of objects per slab page that any slab cache 
> can be configured for which is a compile time limit.
>
> Maybe this is okay, Pekka?
>

If name is not that good -- maybe OO_OBJS_PER_PAGE? :)

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Pekka Enberg - Wed, Oct 22, 2008 at 08:50:56PM +0300]
> Christoph Lameter wrote:
>> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>>
>>> Christoph how about this one?
>>
>> Ok. Looks a bit better but we still have two maxes here
>>
>> s->max which refers to the maximum number of objects per slab page for 
>> a specific slab cache (depends on the runtime configuration). 
>> OO_MAX_OBJS refers to the maximum number of objects per slab page that 
>> any slab cache can be configured for which is a compile time limit.
>>
>> Maybe this is okay, Pekka?
>
> Maybe call the page->objects maximum MAX_OBJS_PER_PAGE as it's not  
> strictly related to the other OO code?
>
> 		Pekka
>

Something like that?

		- Cyrill -
---

 mm/slub.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux-2.6.git/mm/slub.c
===================================================================
--- linux-2.6.git.orig/mm/slub.c	2008-10-22 21:11:26.000000000 +0400
+++ linux-2.6.git/mm/slub.c	2008-10-22 21:57:11.000000000 +0400
@@ -153,6 +153,10 @@
 #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
 #endif
 
+#define OO_SHIFT	16
+#define OO_MASK		((1 << OO_SHIFT) - 1)
+#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
+
 /* Internal SLUB flags */
 #define __OBJECT_POISON		0x80000000 /* Poison object */
 #define __SYSFS_ADD_DEFERRED	0x40000000 /* Not yet visible via sysfs */
@@ -290,7 +294,7 @@ static inline struct kmem_cache_order_ob
 						unsigned long size)
 {
 	struct kmem_cache_order_objects x = {
-		(order << 16) + (PAGE_SIZE << order) / size
+		(order << OO_SHIFT) + (PAGE_SIZE << order) / size
 	};
 
 	return x;
@@ -298,12 +302,12 @@ static inline struct kmem_cache_order_ob
 
 static inline int oo_order(struct kmem_cache_order_objects x)
 {
-	return x.x >> 16;
+	return x.x >> OO_SHIFT;
 }
 
 static inline int oo_objects(struct kmem_cache_order_objects x)
 {
-	return x.x & ((1 << 16) - 1);
+	return x.x & OO_MASK;
 }
 
 #ifdef CONFIG_SLUB_DEBUG
@@ -764,8 +768,8 @@ static int on_freelist(struct kmem_cache
 	}
 
 	max_objects = (PAGE_SIZE << compound_order(page)) / s->size;
-	if (max_objects > 65535)
-		max_objects = 65535;
+	if (max_objects > MAX_OBJS_PER_PAGE)
+		max_objects = MAX_OBJS_PER_PAGE;
 
 	if (page->objects != max_objects) {
 		slab_err(s, page, "Wrong number of objects. Found %d but "
@@ -1819,8 +1823,8 @@ static inline int slab_order(int size, i
 	int rem;
 	int min_order = slub_min_order;
 
-	if ((PAGE_SIZE << min_order) / size > 65535)
-		return get_order(size * 65535) - 1;
+	if ((PAGE_SIZE << min_order) / size > MAX_OBJS_PER_PAGE)
+		return get_order(size * MAX_OBJS_PER_PAGE) - 1;
 
 	for (order = max(min_order,
 				fls(min_objects * size - 1) - PAGE_SHIFT);

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Pekka Enberg]

Cyrill Gorcunov wrote:
> Something like that?
> 
> 		- Cyrill -
> ---
> 
>  mm/slub.c |   18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6.git/mm/slub.c
> ===================================================================
> --- linux-2.6.git.orig/mm/slub.c	2008-10-22 21:11:26.000000000 +0400
> +++ linux-2.6.git/mm/slub.c	2008-10-22 21:57:11.000000000 +0400
> @@ -153,6 +153,10 @@
>  #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long)
>  #endif
>  
> +#define OO_SHIFT	16
> +#define OO_MASK		((1 << OO_SHIFT) - 1)
> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
> +
>  /* Internal SLUB flags */
>  #define __OBJECT_POISON		0x80000000 /* Poison object */
>  #define __SYSFS_ADD_DEFERRED	0x40000000 /* Not yet visible via sysfs */
> @@ -290,7 +294,7 @@ static inline struct kmem_cache_order_ob
>  						unsigned long size)
>  {
>  	struct kmem_cache_order_objects x = {
> -		(order << 16) + (PAGE_SIZE << order) / size
> +		(order << OO_SHIFT) + (PAGE_SIZE << order) / size
>  	};
>  
>  	return x;
> @@ -298,12 +302,12 @@ static inline struct kmem_cache_order_ob
>  
>  static inline int oo_order(struct kmem_cache_order_objects x)
>  {
> -	return x.x >> 16;
> +	return x.x >> OO_SHIFT;
>  }
>  
>  static inline int oo_objects(struct kmem_cache_order_objects x)
>  {
> -	return x.x & ((1 << 16) - 1);
> +	return x.x & OO_MASK;
>  }
>  
>  #ifdef CONFIG_SLUB_DEBUG
> @@ -764,8 +768,8 @@ static int on_freelist(struct kmem_cache
>  	}
>  
>  	max_objects = (PAGE_SIZE << compound_order(page)) / s->size;
> -	if (max_objects > 65535)
> -		max_objects = 65535;
> +	if (max_objects > MAX_OBJS_PER_PAGE)
> +		max_objects = MAX_OBJS_PER_PAGE;
>  
>  	if (page->objects != max_objects) {
>  		slab_err(s, page, "Wrong number of objects. Found %d but "
> @@ -1819,8 +1823,8 @@ static inline int slab_order(int size, i
>  	int rem;
>  	int min_order = slub_min_order;
>  
> -	if ((PAGE_SIZE << min_order) / size > 65535)
> -		return get_order(size * 65535) - 1;
> +	if ((PAGE_SIZE << min_order) / size > MAX_OBJS_PER_PAGE)
> +		return get_order(size * MAX_OBJS_PER_PAGE) - 1;
>  
>  	for (order = max(min_order,
>  				fls(min_objects * size - 1) - PAGE_SHIFT);

Looks good to me. If Christoph ACKs this, please send me a proper patch 
with your sign-off and I'll merge it to slab.git.

		Pekka

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Pekka Enberg wrote:

> Maybe call the page->objects maximum MAX_OBJS_PER_PAGE as it's not strictly 
> related to the other OO code?

Yes. Sounds good.

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> +#define OO_SHIFT	16
> +#define OO_MASK		((1 << OO_SHIFT) - 1)
> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */

This is == OO_MASK right? Otherwise things will break when we change 
OO_SHIFT.

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 11:10:56AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> +#define OO_SHIFT	16
>> +#define OO_MASK		((1 << OO_SHIFT) - 1)
>> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
>
> This is == OO_MASK right? Otherwise things will break when we change  
> OO_SHIFT.
>

Yes, I set it 65535 directly to distinguish it from OO_MASK
meaning not value and point to page.objects since they are
u16. Which meant that is the point where all limits start.
So it we set OO_SHIFT to say 14 it will not be a problem
but if we exceed 16 bits it will break SLUB. Am I right?
(I become scratching the head :)

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> [Christoph Lameter - Wed, Oct 22, 2008 at 11:10:56AM -0700]
>> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>>
>>> +#define OO_SHIFT	16
>>> +#define OO_MASK		((1 << OO_SHIFT) - 1)
>>> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
>>
>> This is == OO_MASK right? Otherwise things will break when we change
>> OO_SHIFT.
>>
>
> Yes, I set it 65535 directly to distinguish it from OO_MASK
> meaning not value and point to page.objects since they are
> u16. Which meant that is the point where all limits start.
> So it we set OO_SHIFT to say 14 it will not be a problem
> but if we exceed 16 bits it will break SLUB. Am I right?
> (I become scratching the head :)

If you set it > 16 then the size of the field in struct page is violated.

So

#define MAX_OBJ_PER_PAGE MIN(1 << bits_in(page.objects) - 1, OO_MASK)

?

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 11:24:58AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> [Christoph Lameter - Wed, Oct 22, 2008 at 11:10:56AM -0700]
>>> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>>>
>>>> +#define OO_SHIFT	16
>>>> +#define OO_MASK		((1 << OO_SHIFT) - 1)
>>>> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
>>>
>>> This is == OO_MASK right? Otherwise things will break when we change
>>> OO_SHIFT.
>>>
>>
>> Yes, I set it 65535 directly to distinguish it from OO_MASK
>> meaning not value and point to page.objects since they are
>> u16. Which meant that is the point where all limits start.
>> So it we set OO_SHIFT to say 14 it will not be a problem
>> but if we exceed 16 bits it will break SLUB. Am I right?
>> (I become scratching the head :)
>
> If you set it > 16 then the size of the field in struct page is violated.
>
> So
>
> #define MAX_OBJ_PER_PAGE MIN(1 << bits_in(page.objects) - 1, OO_MASK)
>
> ?
>
>

Looks really good for me (if it worth anything). But Christoph
doesn't OO_SHIT inspired by u16 too which means we could use
MAX_OBJ_PER_PAGE in form you mentoined but maybe we should define

#define OO_SHIFT bits_in(page.objects) to point out why we use
16 not 14, not 18 or whatever? How do you think?

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 11:24:58AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> [Christoph Lameter - Wed, Oct 22, 2008 at 11:10:56AM -0700]
>>> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>>>
>>>> +#define OO_SHIFT	16
>>>> +#define OO_MASK		((1 << OO_SHIFT) - 1)
>>>> +#define MAX_OBJS_PER_PAGE	65535 /* see struct page.objects */
>>>
>>> This is == OO_MASK right? Otherwise things will break when we change
>>> OO_SHIFT.
>>>
>>
>> Yes, I set it 65535 directly to distinguish it from OO_MASK
>> meaning not value and point to page.objects since they are
>> u16. Which meant that is the point where all limits start.
>> So it we set OO_SHIFT to say 14 it will not be a problem
>> but if we exceed 16 bits it will break SLUB. Am I right?
>> (I become scratching the head :)
>
> If you set it > 16 then the size of the field in struct page is violated.
>
> So
>
> #define MAX_OBJ_PER_PAGE MIN(1 << bits_in(page.objects) - 1, OO_MASK)
>
> ?
>
>

I think the patch becomes more complicated as it should be.
Maybe just use:

#define MAX_OBJ_PER_PAGE 65535 /* since page.objects is u16 */

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> Looks really good for me (if it worth anything). But Christoph
> doesn't OO_SHIT inspired by u16 too which means we could use
> MAX_OBJ_PER_PAGE in form you mentoined but maybe we should define
>
> #define OO_SHIFT bits_in(page.objects) to point out why we use
> 16 not 14, not 18 or whatever? How do you think?


The choice of the bit size in page.objects is determined by the available 
bytes there. The choice of the OO_SHIFT (nice typo there) is determined by 
the use of a 32bit int that we want to cut into two halves.

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Christoph Lameter]

On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:

> I think the patch becomes more complicated as it should be.
> Maybe just use:
>
> #define MAX_OBJ_PER_PAGE 65535 /* since page.objects is u16 */

Ok.

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

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 11:45:55AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> Looks really good for me (if it worth anything). But Christoph
>> doesn't OO_SHIT inspired by u16 too which means we could use
>> MAX_OBJ_PER_PAGE in form you mentoined but maybe we should define
>>
>> #define OO_SHIFT bits_in(page.objects) to point out why we use
>> 16 not 14, not 18 or whatever? How do you think?
>
>
> The choice of the bit size in page.objects is determined by the available 
> bytes there. The choice of the OO_SHIFT (nice typo there) is determined 
> by the use of a 32bit int that we want to cut into two halves.
>

Ah... I see. So wouldn't you mind to just mentoin page.objects in comment
like 'since page.objects is u16' instead of bits_in magic? Anyone who
will (if any) changing page structure is to grep the sources and find
this comment and will fix MAX_OBJS_PER_PAGE definition.

		- Cyrill -

Re: [RFC] SLUB - define OO_ macro instead of hardcoded numbers

[Cyrill Gorcunov]

[Christoph Lameter - Wed, Oct 22, 2008 at 11:49:56AM -0700]
> On Wed, 22 Oct 2008, Cyrill Gorcunov wrote:
>
>> I think the patch becomes more complicated as it should be.
>> Maybe just use:
>>
>> #define MAX_OBJ_PER_PAGE 65535 /* since page.objects is u16 */
>
> Ok.
>
> Acked-by: Christoph Lameter <cl@linux-foundation.org>
>

Thanks for review -- will make normal patch shortly and
send it to Pekka. Thanks!

		- Cyrill -