[PATCH] slab: add a check for the first kmem_cache not to be destroyed

[PATCH] slab: add a check for the first kmem_cache not to be destroyed

[Kyunghwan Kwon]

The first kmem_cache created at booting up is supposed neither mergeable
nor destroyable but was possible to destroy. So prevent it.

Signed-off-by: Kyunghwan Kwon <kwon@toanyone.net>
---
 mm/slab_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 1dfc209..2d30ace 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
 	bool need_rcu_barrier = false;
 	int err;
 
-	if (unlikely(!s))
+	if (unlikely(!s) || s->refcount == -1)
 		return;
 
 	get_online_cpus();
-- 
2.9.3 (Apple Git-75)


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

Re: [PATCH] slab: add a check for the first kmem_cache not to be destroyed

[Joonsoo Kim]

On Mon, Jan 16, 2017 at 04:04:59PM +0900, Kyunghwan Kwon wrote:
> The first kmem_cache created at booting up is supposed neither mergeable
> nor destroyable but was possible to destroy. So prevent it.
> 
> Signed-off-by: Kyunghwan Kwon <kwon@toanyone.net>
> ---
>  mm/slab_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 1dfc209..2d30ace 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
>  	bool need_rcu_barrier = false;
>  	int err;
>  
> -	if (unlikely(!s))
> +	if (unlikely(!s) || s->refcount == -1)
>  		return;

Hello, Kyunghwan.

Few lines below, s->refcount is checked.

if (s->refcount)
        goto unlock;

Am I missing something?

Thanks.

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

Re: [PATCH] slab: add a check for the first kmem_cache not to be destroyed

[kwon]

> On Jan 17, 2017, at 10:33 AM, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> 
> On Mon, Jan 16, 2017 at 04:04:59PM +0900, Kyunghwan Kwon wrote:
>> The first kmem_cache created at booting up is supposed neither mergeable
>> nor destroyable but was possible to destroy. So prevent it.
>> 
>> Signed-off-by: Kyunghwan Kwon <kwon@toanyone.net>
>> ---
>> mm/slab_common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>> index 1dfc209..2d30ace 100644
>> --- a/mm/slab_common.c
>> +++ b/mm/slab_common.c
>> @@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
>> 	bool need_rcu_barrier = false;
>> 	int err;
>> 
>> -	if (unlikely(!s))
>> +	if (unlikely(!s) || s->refcount == -1)
>> 		return;
> 
> Hello, Kyunghwan.
> 
> Few lines below, s->refcount is checked.
> 
> if (s->refcount)
>        goto unlock;
> 
> Am I missing something?
> 
> Thanks.

Hello, Joonsoo.

In case it is called the number of int size times. refcount would finally reach
to 0 since decreased every time the function called.

When refcount is -1, the count will not change in the patch so no lock would be
need to be taken prior, I believe.

Thanks.
--
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>

Re: [PATCH] slab: add a check for the first kmem_cache not to be destroyed

[David Rientjes]

On Tue, 17 Jan 2017, kwon wrote:

> >> diff --git a/mm/slab_common.c b/mm/slab_common.c
> >> index 1dfc209..2d30ace 100644
> >> --- a/mm/slab_common.c
> >> +++ b/mm/slab_common.c
> >> @@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
> >> 	bool need_rcu_barrier = false;
> >> 	int err;
> >> 
> >> -	if (unlikely(!s))
> >> +	if (unlikely(!s) || s->refcount == -1)
> >> 		return;
> > 
> > Hello, Kyunghwan.
> > 
> > Few lines below, s->refcount is checked.
> > 
> > if (s->refcount)
> >        goto unlock;
> > 
> > Am I missing something?
> > 
> > Thanks.
> 
> Hello, Joonsoo.
> 
> In case it is called the number of int size times. refcount would finally reach
> to 0 since decreased every time the function called.
> 

The only thing using create_boot_cache() should be the slab implementation 
itself, so I don't think we need to protect ourselves from doing something 
like kmem_cache_destroy(kmem_cache) or 
kmem_cache_destroy(kmem_cache_node) even a single time.

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

Re: [PATCH] slab: add a check for the first kmem_cache not to be destroyed

[kwon]

> On Jan 18, 2017, at 7:54 AM, David Rientjes <rientjes@google.com> wrote:
> 
> On Tue, 17 Jan 2017, kwon wrote:
> 
>>>> diff --git a/mm/slab_common.c b/mm/slab_common.c
>>>> index 1dfc209..2d30ace 100644
>>>> --- a/mm/slab_common.c
>>>> +++ b/mm/slab_common.c
>>>> @@ -744,7 +744,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
>>>> 	bool need_rcu_barrier = false;
>>>> 	int err;
>>>> 
>>>> -	if (unlikely(!s))
>>>> +	if (unlikely(!s) || s->refcount == -1)
>>>> 		return;
>>> 
>>> Hello, Kyunghwan.
>>> 
>>> Few lines below, s->refcount is checked.
>>> 
>>> if (s->refcount)
>>>       goto unlock;
>>> 
>>> Am I missing something?
>>> 
>>> Thanks.
>> 
>> Hello, Joonsoo.
>> 
>> In case it is called the number of int size times. refcount would finally reach
>> to 0 since decreased every time the function called.
>> 
> 
> The only thing using create_boot_cache() should be the slab implementation 
> itself, so I don't think we need to protect ourselves from doing something 
> like kmem_cache_destroy(kmem_cache) or 
> kmem_cache_destroy(kmem_cache_node) even a single time.

Agreed. I was aware of that though, I thought it would make its logic firm not
giving performance disadvantages. Sorry for distraction.
--
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>