[RFC PATCH] mm/zsmalloc: remove unnecessary check

[RFC PATCH] mm/zsmalloc: remove unnecessary check

[Mahendran Ganesh]

ZS_SIZE_CLASSES is calc by:
  ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)

So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
  size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
will not be greater than ZS_MAX_ALLOC_SIZE

This patch removes the unnecessary check.

Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
---
 mm/zsmalloc.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index b3b57ef..f2279e2 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -973,8 +973,6 @@ struct zs_pool *zs_create_pool(gfp_t flags)
 		struct size_class *prev_class;
 
 		size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA;
-		if (size > ZS_MAX_ALLOC_SIZE)
-			size = ZS_MAX_ALLOC_SIZE;
 		pages_per_zspage = get_pages_per_zspage(size);
 
 		/*
-- 
1.7.9.5

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Minchan Kim]

On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
> ZS_SIZE_CLASSES is calc by:
>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
> 
> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
> will not be greater than ZS_MAX_ALLOC_SIZE
> 
> This patch removes the unnecessary check.

It depends on ZS_MIN_ALLOC_SIZE.
For example, we would change min to 8 but MAX is still 4096.
ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
which exceeds the max.

> 
> Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
> ---
>  mm/zsmalloc.c |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index b3b57ef..f2279e2 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -973,8 +973,6 @@ struct zs_pool *zs_create_pool(gfp_t flags)
>  		struct size_class *prev_class;
>  
>  		size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA;
> -		if (size > ZS_MAX_ALLOC_SIZE)
> -			size = ZS_MAX_ALLOC_SIZE;
>  		pages_per_zspage = get_pages_per_zspage(size);
>  
>  		/*
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Ganesh Mahendran]

Hello

2014-11-21 11:54 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
>> ZS_SIZE_CLASSES is calc by:
>>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
>>
>> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
>>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
>> will not be greater than ZS_MAX_ALLOC_SIZE
>>
>> This patch removes the unnecessary check.
>
> It depends on ZS_MIN_ALLOC_SIZE.
> For example, we would change min to 8 but MAX is still 4096.
> ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
> which exceeds the max.
Here, 4088 is less than MAX(4096).

ZS_SIZE_CLASSES = (MAX - MIN) / Delta + 1
So, I think the value of
    MIN + (ZS_SIZE_CLASSES - 1) * Delta =
    MIN + ((MAX - MIN) / Delta) * Delta =
    MAX
will not exceed the MAX

Thanks.

>
>>
>> Signed-off-by: Mahendran Ganesh <opensource.ganesh@gmail.com>
>> ---
>>  mm/zsmalloc.c |    2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index b3b57ef..f2279e2 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -973,8 +973,6 @@ struct zs_pool *zs_create_pool(gfp_t flags)
>>               struct size_class *prev_class;
>>
>>               size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA;
>> -             if (size > ZS_MAX_ALLOC_SIZE)
>> -                     size = ZS_MAX_ALLOC_SIZE;
>>               pages_per_zspage = get_pages_per_zspage(size);
>>
>>               /*
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
> --
> Kind regards,
> Minchan Kim

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Minchan Kim]

On Fri, Nov 21, 2014 at 01:33:26PM +0800, Ganesh Mahendran wrote:
> Hello
> 
> 2014-11-21 11:54 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> > On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
> >> ZS_SIZE_CLASSES is calc by:
> >>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
> >>
> >> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
> >>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
> >> will not be greater than ZS_MAX_ALLOC_SIZE
> >>
> >> This patch removes the unnecessary check.
> >
> > It depends on ZS_MIN_ALLOC_SIZE.
> > For example, we would change min to 8 but MAX is still 4096.
> > ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
> > which exceeds the max.
> Here, 4088 is less than MAX(4096).
> 
> ZS_SIZE_CLASSES = (MAX - MIN) / Delta + 1
> So, I think the value of
>     MIN + (ZS_SIZE_CLASSES - 1) * Delta =
>     MIN + ((MAX - MIN) / Delta) * Delta =
>     MAX
> will not exceed the MAX

You're right. It was complext math for me.
I should go back to elementary school.

Thanks!

Acked-by: Minchan Kim <minchan@kernel.org>

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Minchan Kim]

On Fri, Nov 21, 2014 at 06:48:49AM +0000, Minchan Kim wrote:
> On Fri, Nov 21, 2014 at 01:33:26PM +0800, Ganesh Mahendran wrote:
> > Hello
> > 
> > 2014-11-21 11:54 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> > > On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
> > >> ZS_SIZE_CLASSES is calc by:
> > >>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
> > >>
> > >> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
> > >>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
> > >> will not be greater than ZS_MAX_ALLOC_SIZE
> > >>
> > >> This patch removes the unnecessary check.
> > >
> > > It depends on ZS_MIN_ALLOC_SIZE.
> > > For example, we would change min to 8 but MAX is still 4096.
> > > ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
> > > which exceeds the max.
> > Here, 4088 is less than MAX(4096).
> > 
> > ZS_SIZE_CLASSES = (MAX - MIN) / Delta + 1
> > So, I think the value of
> >     MIN + (ZS_SIZE_CLASSES - 1) * Delta =
> >     MIN + ((MAX - MIN) / Delta) * Delta =
> >     MAX
> > will not exceed the MAX
> 
> You're right. It was complext math for me.
> I should go back to elementary school.
> 
> Thanks!
> 
> Acked-by: Minchan Kim <minchan@kernel.org>

I catch a nasty cold but above my poor math makes me think more.
ZS_SIZE_CLASSES is broken. In above my example, current code cannot
allocate 4096 size class so we should correct ZS_SIZE_CLASSES
at first.

zs_size_classes = zs_max - zs_min / delta + 1;
if ((zs_max - zs_min) % delta)
	zs_size_classes += 1;

Then, we need to code piece you removed.
As well, we need to fix below.

- area->vm_buf = (char *)__get_free_page(GFP_KERNEL);
+ area->vm_buf = kmalloc(ZS_MAX_ALLOC_SIZE);

Hope I am sane in this time :(

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Ganesh Mahendran]

Hello Minchan

2014-11-21 18:32 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> On Fri, Nov 21, 2014 at 06:48:49AM +0000, Minchan Kim wrote:
>> On Fri, Nov 21, 2014 at 01:33:26PM +0800, Ganesh Mahendran wrote:
>> > Hello
>> >
>> > 2014-11-21 11:54 GMT+08:00 Minchan Kim <minchan@kernel.org>:
>> > > On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
>> > >> ZS_SIZE_CLASSES is calc by:
>> > >>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
>> > >>
>> > >> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
>> > >>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
>> > >> will not be greater than ZS_MAX_ALLOC_SIZE
>> > >>
>> > >> This patch removes the unnecessary check.
>> > >
>> > > It depends on ZS_MIN_ALLOC_SIZE.
>> > > For example, we would change min to 8 but MAX is still 4096.
>> > > ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
>> > > which exceeds the max.
>> > Here, 4088 is less than MAX(4096).
>> >
>> > ZS_SIZE_CLASSES = (MAX - MIN) / Delta + 1
>> > So, I think the value of
>> >     MIN + (ZS_SIZE_CLASSES - 1) * Delta =
>> >     MIN + ((MAX - MIN) / Delta) * Delta =
>> >     MAX
>> > will not exceed the MAX
>>
>> You're right. It was complext math for me.
>> I should go back to elementary school.
>>
>> Thanks!
>>
>> Acked-by: Minchan Kim <minchan@kernel.org>
>
> I catch a nasty cold but above my poor math makes me think more.
> ZS_SIZE_CLASSES is broken. In above my example, current code cannot
> allocate 4096 size class so we should correct ZS_SIZE_CLASSES
> at first.
>
> zs_size_classes = zs_max - zs_min / delta + 1;
> if ((zs_max - zs_min) % delta)
>         zs_size_classes += 1;
Yes, you are right.
When the zs_min is less than delta, we can not allocate PAGE_SIZE size class.

>
> Then, we need to code piece you removed.
> As well, we need to fix below.
>
> - area->vm_buf = (char *)__get_free_page(GFP_KERNEL);
> + area->vm_buf = kmalloc(ZS_MAX_ALLOC_SIZE);
If our purpose is to allocate the max obj size as len of PAGE_SIZE, we
do not need to
change this line. Since the ZS_MAX_ALLOC_SIZE will always be PAGE_SIZE

Thanks.

>
> Hope I am sane in this time :(

Re: [RFC PATCH] mm/zsmalloc: remove unnecessary check

[Minchan Kim]

Hello Ganesh,

On Fri, Nov 21, 2014 at 10:56:10PM +0800, Ganesh Mahendran wrote:
> Hello Minchan
> 
> 2014-11-21 18:32 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> > On Fri, Nov 21, 2014 at 06:48:49AM +0000, Minchan Kim wrote:
> >> On Fri, Nov 21, 2014 at 01:33:26PM +0800, Ganesh Mahendran wrote:
> >> > Hello
> >> >
> >> > 2014-11-21 11:54 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> >> > > On Thu, Nov 20, 2014 at 09:21:56PM +0800, Mahendran Ganesh wrote:
> >> > >> ZS_SIZE_CLASSES is calc by:
> >> > >>   ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1)
> >> > >>
> >> > >> So when i is in [0, ZS_SIZE_CLASSES - 1), the size:
> >> > >>   size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA
> >> > >> will not be greater than ZS_MAX_ALLOC_SIZE
> >> > >>
> >> > >> This patch removes the unnecessary check.
> >> > >
> >> > > It depends on ZS_MIN_ALLOC_SIZE.
> >> > > For example, we would change min to 8 but MAX is still 4096.
> >> > > ZS_SIZE_CLASSES is (4096 - 8) / 16 + 1 = 256 so 8 + 255 * 16 = 4088,
> >> > > which exceeds the max.
> >> > Here, 4088 is less than MAX(4096).
> >> >
> >> > ZS_SIZE_CLASSES = (MAX - MIN) / Delta + 1
> >> > So, I think the value of
> >> >     MIN + (ZS_SIZE_CLASSES - 1) * Delta =
> >> >     MIN + ((MAX - MIN) / Delta) * Delta =
> >> >     MAX
> >> > will not exceed the MAX
> >>
> >> You're right. It was complext math for me.
> >> I should go back to elementary school.
> >>
> >> Thanks!
> >>
> >> Acked-by: Minchan Kim <minchan@kernel.org>
> >
> > I catch a nasty cold but above my poor math makes me think more.
> > ZS_SIZE_CLASSES is broken. In above my example, current code cannot
> > allocate 4096 size class so we should correct ZS_SIZE_CLASSES
> > at first.
> >
> > zs_size_classes = zs_max - zs_min / delta + 1;
> > if ((zs_max - zs_min) % delta)
> >         zs_size_classes += 1;
> Yes, you are right.
> When the zs_min is less than delta, we can not allocate PAGE_SIZE size class.
> 
> >
> > Then, we need to code piece you removed.
> > As well, we need to fix below.
> >
> > - area->vm_buf = (char *)__get_free_page(GFP_KERNEL);
> > + area->vm_buf = kmalloc(ZS_MAX_ALLOC_SIZE);
> If our purpose is to allocate the max obj size as len of PAGE_SIZE, we
> do not need to
> change this line. Since the ZS_MAX_ALLOC_SIZE will always be PAGE_SIZE

No, please don't assume ZS_MAX_ALLOC_SIZE is PAGE_SIZE forever.
Some reason could make buffer larger than PAGE_SIZE.
For example, we might put allocator's metadata into each object's head/tail.

-- 
Kind regards,
Minchan Kim