2014-11-20 13:22:09

by Ganesh Mahendran

[permalink] [raw]
Subject: [RFC PATCH] mm/zsmalloc: remove unnecessary check

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 <[email protected]>
---
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


2014-11-21 03:54:37

by Minchan Kim

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

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 <[email protected]>
> ---
> 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 [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
Kind regards,
Minchan Kim

2014-11-21 05:33:28

by Ganesh Mahendran

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

Hello

2014-11-21 11:54 GMT+08:00 Minchan Kim <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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 [email protected]. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>
> --
> Kind regards,
> Minchan Kim

2014-11-21 06:48:21

by Minchan Kim

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

On Fri, Nov 21, 2014 at 01:33:26PM +0800, Ganesh Mahendran wrote:
> Hello
>
> 2014-11-21 11:54 GMT+08:00 Minchan Kim <[email protected]>:
> > 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 <[email protected]>

2014-11-21 10:32:46

by Minchan Kim

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

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 <[email protected]>:
> > > 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 <[email protected]>

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 :(

2014-11-21 14:56:12

by Ganesh Mahendran

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

Hello Minchan

2014-11-21 18:32 GMT+08:00 Minchan Kim <[email protected]>:
> 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 <[email protected]>:
>> > > 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 <[email protected]>
>
> 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 :(

2014-11-24 07:55:29

by Minchan Kim

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

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 <[email protected]>:
> > 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 <[email protected]>:
> >> > > 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 <[email protected]>
> >
> > 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