From: zijun_hu <zijun_hu@zoho.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
zijun_hu@htc.com, tj@kernel.org, akpm@linux-foundation.org,
cl@linux.com
Subject: Re: [RFC PATCH 1/1] mm/percpu.c: fix memory leakage issue when allocate a odd alignment area
Date: Wed, 12 Oct 2016 16:44:31 +0800 [thread overview]
Message-ID: <57FDF7EF.6070606@zoho.com> (raw)
In-Reply-To: <20161012082538.GC17128@dhcp22.suse.cz>
On 10/12/2016 04:25 PM, Michal Hocko wrote:
> On Wed 12-10-16 15:24:33, zijun_hu wrote:
>> On 10/12/2016 02:53 PM, Michal Hocko wrote:
>>> On Wed 12-10-16 08:28:17, zijun_hu wrote:
>>>> On 2016/10/12 1:22, Michal Hocko wrote:
>>>>> On Tue 11-10-16 21:24:50, zijun_hu wrote:
>>>>>> From: zijun_hu <zijun_hu@htc.com>
>>>>>>
>> should we have a generic discussion whether such patches which considers
>> many boundary or rare conditions are necessary.
>
> In general, I believe that kernel internal interfaces which have no
> userspace exposure shouldn't be cluttered with sanity checks.
>
you are right and i agree with you. but there are many internal interfaces
perform sanity checks in current linux sources
>> i found the following code segments in mm/vmalloc.c
>> static struct vmap_area *alloc_vmap_area(unsigned long size,
>> unsigned long align,
>> unsigned long vstart, unsigned long vend,
>> int node, gfp_t gfp_mask)
>> {
>> ...
>>
>> BUG_ON(!size);
>> BUG_ON(offset_in_page(size));
>> BUG_ON(!is_power_of_2(align));
>
> See a recent Linus rant about BUG_ONs. These BUG_ONs are quite old and
> from a quick look they are even unnecessary. So rather than adding more
> of those, I think removing those that are not needed is much more
> preferred.
>
i notice that, and the above code segments is used to illustrate that
input parameter checking is necessary sometimes
>> should we make below declarations as conventions
>> 1) when we say 'alignment', it means align to a power of 2 value
>> for example, aligning value @v to @b implicit @v is power of 2
>> , align 10 to 4 is 12
>
> alignment other than power-of-two makes only very limited sense to me.
>
you are right and i agree with you.
>> 2) when we say 'round value @v up/down to boundary @b', it means the
>> result is a times of @b, it don't requires @b is a power of 2
>
i will write to linus to ask for opinions whether we should declare
the meaning of 'align' and 'round up/down' formally and whether such
patches are necessary
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: zijun_hu <zijun_hu@zoho.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
zijun_hu@htc.com, tj@kernel.org, akpm@linux-foundation.org,
cl@linux.com
Subject: Re: [RFC PATCH 1/1] mm/percpu.c: fix memory leakage issue when allocate a odd alignment area
Date: Wed, 12 Oct 2016 16:44:31 +0800 [thread overview]
Message-ID: <57FDF7EF.6070606@zoho.com> (raw)
In-Reply-To: <20161012082538.GC17128@dhcp22.suse.cz>
On 10/12/2016 04:25 PM, Michal Hocko wrote:
> On Wed 12-10-16 15:24:33, zijun_hu wrote:
>> On 10/12/2016 02:53 PM, Michal Hocko wrote:
>>> On Wed 12-10-16 08:28:17, zijun_hu wrote:
>>>> On 2016/10/12 1:22, Michal Hocko wrote:
>>>>> On Tue 11-10-16 21:24:50, zijun_hu wrote:
>>>>>> From: zijun_hu <zijun_hu@htc.com>
>>>>>>
>> should we have a generic discussion whether such patches which considers
>> many boundary or rare conditions are necessary.
>
> In general, I believe that kernel internal interfaces which have no
> userspace exposure shouldn't be cluttered with sanity checks.
>
you are right and i agree with you. but there are many internal interfaces
perform sanity checks in current linux sources
>> i found the following code segments in mm/vmalloc.c
>> static struct vmap_area *alloc_vmap_area(unsigned long size,
>> unsigned long align,
>> unsigned long vstart, unsigned long vend,
>> int node, gfp_t gfp_mask)
>> {
>> ...
>>
>> BUG_ON(!size);
>> BUG_ON(offset_in_page(size));
>> BUG_ON(!is_power_of_2(align));
>
> See a recent Linus rant about BUG_ONs. These BUG_ONs are quite old and
> from a quick look they are even unnecessary. So rather than adding more
> of those, I think removing those that are not needed is much more
> preferred.
>
i notice that, and the above code segments is used to illustrate that
input parameter checking is necessary sometimes
>> should we make below declarations as conventions
>> 1) when we say 'alignment', it means align to a power of 2 value
>> for example, aligning value @v to @b implicit @v is power of 2
>> , align 10 to 4 is 12
>
> alignment other than power-of-two makes only very limited sense to me.
>
you are right and i agree with you.
>> 2) when we say 'round value @v up/down to boundary @b', it means the
>> result is a times of @b, it don't requires @b is a power of 2
>
i will write to linus to ask for opinions whether we should declare
the meaning of 'align' and 'round up/down' formally and whether such
patches are necessary
next prev parent reply other threads:[~2016-10-12 8:45 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-11 13:24 [RFC PATCH 1/1] mm/percpu.c: fix memory leakage issue when allocate a odd alignment area zijun_hu
2016-10-11 13:24 ` zijun_hu
2016-10-11 17:22 ` Michal Hocko
2016-10-11 17:22 ` Michal Hocko
2016-10-12 0:28 ` zijun_hu
2016-10-12 0:28 ` zijun_hu
2016-10-12 6:53 ` Michal Hocko
2016-10-12 6:53 ` Michal Hocko
2016-10-12 7:20 ` zijun_hu
2016-10-12 7:20 ` zijun_hu
2016-10-12 7:24 ` zijun_hu
2016-10-12 7:24 ` zijun_hu
2016-10-12 8:25 ` Michal Hocko
2016-10-12 8:25 ` Michal Hocko
2016-10-12 8:44 ` zijun_hu [this message]
2016-10-12 8:44 ` zijun_hu
2016-10-12 9:54 ` Michal Hocko
2016-10-12 9:54 ` Michal Hocko
2016-10-12 9:59 ` zijun_hu
2016-10-12 9:59 ` zijun_hu
2016-10-13 23:31 ` Tejun Heo
2016-10-13 23:31 ` Tejun Heo
2016-10-14 0:23 ` zijun_hu
2016-10-14 0:23 ` zijun_hu
2016-10-14 0:28 ` Tejun Heo
2016-10-14 0:28 ` Tejun Heo
2016-10-14 0:58 ` zijun_hu
2016-10-14 0:58 ` zijun_hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57FDF7EF.6070606@zoho.com \
--to=zijun_hu@zoho.com \
--cc=akpm@linux-foundation.org \
--cc=cl@linux.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=tj@kernel.org \
--cc=zijun_hu@htc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.