* [PATCH] mm, slab: remove unnecessary unlikely()
@ 2018-11-04 12:50 Yangtao Li
2018-11-05 10:18 ` Vlastimil Babka
2018-11-07 22:55 ` David Rientjes
0 siblings, 2 replies; 5+ messages in thread
From: Yangtao Li @ 2018-11-04 12:50 UTC (permalink / raw)
To: cl, penberg, rientjes, iamjoonsoo.kim, akpm
Cc: linux-mm, linux-kernel, Yangtao Li
WARN_ON() already contains an unlikely(), so it's not necessary to use
unlikely.
Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
---
mm/slab_common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 7eb8dc136c1c..4f54684f5435 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1029,10 +1029,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
index = size_index[size_index_elem(size)];
} else {
- if (unlikely(size > KMALLOC_MAX_CACHE_SIZE)) {
- WARN_ON(1);
+ if (WARN_ON(size > KMALLOC_MAX_CACHE_SIZE))
return NULL;
- }
index = fls(size - 1);
}
--
2.17.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm, slab: remove unnecessary unlikely()
2018-11-04 12:50 [PATCH] mm, slab: remove unnecessary unlikely() Yangtao Li
@ 2018-11-05 10:18 ` Vlastimil Babka
2018-11-06 5:38 ` Dmitry Vyukov
2018-11-07 22:55 ` David Rientjes
1 sibling, 1 reply; 5+ messages in thread
From: Vlastimil Babka @ 2018-11-05 10:18 UTC (permalink / raw)
To: Yangtao Li, cl, penberg, rientjes, iamjoonsoo.kim, akpm
Cc: linux-mm, linux-kernel, Dmitry Vyukov
+CC Dmitry
On 11/4/18 1:50 PM, Yangtao Li wrote:
> WARN_ON() already contains an unlikely(), so it's not necessary to use
> unlikely.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Maybe also change it back to WARN_ON_ONCE? I already considered it while
reviewing Dmitry's patch and wasn't sure. Now I think that what can
happen is that either a kernel bug is introduced that _ONCE is enough to
catch (two separate bugs introduced to both hit this would be rare, and
in that case the second one will be reported after the first one is
fixed), or this gets called with a user-supplied value, and then we want
to avoid spamming dmesg with multiple warnings that the user could
trigger at will.
> ---
> mm/slab_common.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 7eb8dc136c1c..4f54684f5435 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1029,10 +1029,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
>
> index = size_index[size_index_elem(size)];
> } else {
> - if (unlikely(size > KMALLOC_MAX_CACHE_SIZE)) {
> - WARN_ON(1);
> + if (WARN_ON(size > KMALLOC_MAX_CACHE_SIZE))
> return NULL;
> - }
> index = fls(size - 1);
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm, slab: remove unnecessary unlikely()
2018-11-05 10:18 ` Vlastimil Babka
@ 2018-11-06 5:38 ` Dmitry Vyukov
2018-11-06 8:54 ` Vlastimil Babka
0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Vyukov @ 2018-11-06 5:38 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Yangtao Li, Christoph Lameter, Pekka Enberg, David Rientjes,
Joonsoo Kim, Andrew Morton, Linux-MM, LKML
On Mon, Nov 5, 2018 at 11:18 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
> +CC Dmitry
>
> On 11/4/18 1:50 PM, Yangtao Li wrote:
>> WARN_ON() already contains an unlikely(), so it's not necessary to use
>> unlikely.
>>
>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>
> Maybe also change it back to WARN_ON_ONCE? I already considered it while
> reviewing Dmitry's patch and wasn't sure. Now I think that what can
> happen is that either a kernel bug is introduced that _ONCE is enough to
> catch (two separate bugs introduced to both hit this would be rare, and
> in that case the second one will be reported after the first one is
> fixed), or this gets called with a user-supplied value, and then we want
> to avoid spamming dmesg with multiple warnings that the user could
> trigger at will.
If you asking me, I am fine both changes.
I was mainly interested in removing the bogus warnings that actually fire.
>> ---
>> mm/slab_common.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 7eb8dc136c1c..4f54684f5435 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -1029,10 +1029,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
>>
>> index = size_index[size_index_elem(size)];
>> } else {
>> - if (unlikely(size > KMALLOC_MAX_CACHE_SIZE)) {
>> - WARN_ON(1);
>> + if (WARN_ON(size > KMALLOC_MAX_CACHE_SIZE))
>> return NULL;
>> - }
>> index = fls(size - 1);
>> }
>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm, slab: remove unnecessary unlikely()
2018-11-06 5:38 ` Dmitry Vyukov
@ 2018-11-06 8:54 ` Vlastimil Babka
0 siblings, 0 replies; 5+ messages in thread
From: Vlastimil Babka @ 2018-11-06 8:54 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Yangtao Li, Christoph Lameter, Pekka Enberg, David Rientjes,
Joonsoo Kim, Andrew Morton, Linux-MM, LKML
On 11/6/18 6:38 AM, Dmitry Vyukov wrote:
> On Mon, Nov 5, 2018 at 11:18 AM, Vlastimil Babka <vbabka@suse.cz> wrote:
>> +CC Dmitry
>>
>> On 11/4/18 1:50 PM, Yangtao Li wrote:
>>> WARN_ON() already contains an unlikely(), so it's not necessary to use
>>> unlikely.
>>>
>>> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
>>
>> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>>
>> Maybe also change it back to WARN_ON_ONCE? I already considered it while
>> reviewing Dmitry's patch and wasn't sure. Now I think that what can
>> happen is that either a kernel bug is introduced that _ONCE is enough to
>> catch (two separate bugs introduced to both hit this would be rare, and
>> in that case the second one will be reported after the first one is
>> fixed), or this gets called with a user-supplied value, and then we want
>> to avoid spamming dmesg with multiple warnings that the user could
>> trigger at will.
>
>
> If you asking me, I am fine both changes.
> I was mainly interested in removing the bogus warnings that actually fire.
OK thanks. Andrew can you update the patch to WARN_ON_ONCE?
Changelog addition:
Also change WARN_ON() back to WARN_ON_ONCE() to avoid potentially
spamming dmesg with user-triggerable large allocations.
>
>>> ---
>>> mm/slab_common.c | 4 +---
>>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>> index 7eb8dc136c1c..4f54684f5435 100644
>>> --- a/mm/slab_common.c
>>> +++ b/mm/slab_common.c
>>> @@ -1029,10 +1029,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
>>>
>>> index = size_index[size_index_elem(size)];
>>> } else {
>>> - if (unlikely(size > KMALLOC_MAX_CACHE_SIZE)) {
>>> - WARN_ON(1);
>>> + if (WARN_ON(size > KMALLOC_MAX_CACHE_SIZE))
>>> return NULL;
>>> - }
>>> index = fls(size - 1);
>>> }
>>>
>>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm, slab: remove unnecessary unlikely()
2018-11-04 12:50 [PATCH] mm, slab: remove unnecessary unlikely() Yangtao Li
2018-11-05 10:18 ` Vlastimil Babka
@ 2018-11-07 22:55 ` David Rientjes
1 sibling, 0 replies; 5+ messages in thread
From: David Rientjes @ 2018-11-07 22:55 UTC (permalink / raw)
To: Yangtao Li; +Cc: cl, penberg, iamjoonsoo.kim, akpm, linux-mm, linux-kernel
On Sun, 4 Nov 2018, Yangtao Li wrote:
> WARN_ON() already contains an unlikely(), so it's not necessary to use
> unlikely.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
Acked-by: David Rientjes <rientjes@google.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-07 22:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-04 12:50 [PATCH] mm, slab: remove unnecessary unlikely() Yangtao Li
2018-11-05 10:18 ` Vlastimil Babka
2018-11-06 5:38 ` Dmitry Vyukov
2018-11-06 8:54 ` Vlastimil Babka
2018-11-07 22:55 ` David Rientjes
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.