[PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Salah Triki]

kmem_cache_destroy can be called with NULL values. Thus, the checks that
precede the calls are useless.

Signed-off-by: Salah Triki <salah.triki@acm.org>
---
 fs/jffs2/malloc.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
index b8fd651..ce11897 100644
--- a/fs/jffs2/malloc.c
+++ b/fs/jffs2/malloc.c
@@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
 
 void jffs2_destroy_slab_caches(void)
 {
-	if(full_dnode_slab)
-		kmem_cache_destroy(full_dnode_slab);
-	if(raw_dirent_slab)
-		kmem_cache_destroy(raw_dirent_slab);
-	if(raw_inode_slab)
-		kmem_cache_destroy(raw_inode_slab);
-	if(tmp_dnode_info_slab)
-		kmem_cache_destroy(tmp_dnode_info_slab);
-	if(raw_node_ref_slab)
-		kmem_cache_destroy(raw_node_ref_slab);
-	if(node_frag_slab)
-		kmem_cache_destroy(node_frag_slab);
-	if(inode_cache_slab)
-		kmem_cache_destroy(inode_cache_slab);
+	kmem_cache_destroy(full_dnode_slab);
+	kmem_cache_destroy(raw_dirent_slab);
+	kmem_cache_destroy(raw_inode_slab);
+	kmem_cache_destroy(tmp_dnode_info_slab);
+	kmem_cache_destroy(raw_node_ref_slab);
+	kmem_cache_destroy(node_frag_slab);
+	kmem_cache_destroy(inode_cache_slab);
 #ifdef CONFIG_JFFS2_FS_XATTR
-	if (xattr_datum_cache)
-		kmem_cache_destroy(xattr_datum_cache);
-	if (xattr_ref_cache)
-		kmem_cache_destroy(xattr_ref_cache);
+	kmem_cache_destroy(xattr_datum_cache);
+	kmem_cache_destroy(xattr_ref_cache);
 #endif
 }
 
-- 
1.9.1

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Richard Weinberger]

On Tue, Aug 4, 2015 at 9:46 PM, Salah Triki <salah.triki@acm.org> wrote:
> kmem_cache_destroy can be called with NULL values. Thus, the checks that
> precede the calls are useless.

Since when?
I don't see a !NULL check in kmem_cache_destroy().

-- 
Thanks,
//richard

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Brian Norris]

On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> kmem_cache_destroy can be called with NULL values. Thus, the checks that
> precede the calls are useless.

This isn't *currently* true. Only after a bit of work did I find that
this is queued to change in the next release -- akpm has a patch:

commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date:   Thu Jul 30 09:55:36 2015 +1000

    mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()

Please provide the appropriate context next time. And for the moment, I will
not apply this, because this does not work on the current MTD development tree.
Try your patch bomb on the next release, as it's not worth my time to
cross-merge a -next branch just for a "cleanup".

Brian

> Signed-off-by: Salah Triki <salah.triki@acm.org>
> ---
>  fs/jffs2/malloc.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> index b8fd651..ce11897 100644
> --- a/fs/jffs2/malloc.c
> +++ b/fs/jffs2/malloc.c
> @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
>  
>  void jffs2_destroy_slab_caches(void)
>  {
> -	if(full_dnode_slab)
> -		kmem_cache_destroy(full_dnode_slab);
> -	if(raw_dirent_slab)
> -		kmem_cache_destroy(raw_dirent_slab);
> -	if(raw_inode_slab)
> -		kmem_cache_destroy(raw_inode_slab);
> -	if(tmp_dnode_info_slab)
> -		kmem_cache_destroy(tmp_dnode_info_slab);
> -	if(raw_node_ref_slab)
> -		kmem_cache_destroy(raw_node_ref_slab);
> -	if(node_frag_slab)
> -		kmem_cache_destroy(node_frag_slab);
> -	if(inode_cache_slab)
> -		kmem_cache_destroy(inode_cache_slab);
> +	kmem_cache_destroy(full_dnode_slab);
> +	kmem_cache_destroy(raw_dirent_slab);
> +	kmem_cache_destroy(raw_inode_slab);
> +	kmem_cache_destroy(tmp_dnode_info_slab);
> +	kmem_cache_destroy(raw_node_ref_slab);
> +	kmem_cache_destroy(node_frag_slab);
> +	kmem_cache_destroy(inode_cache_slab);
>  #ifdef CONFIG_JFFS2_FS_XATTR
> -	if (xattr_datum_cache)
> -		kmem_cache_destroy(xattr_datum_cache);
> -	if (xattr_ref_cache)
> -		kmem_cache_destroy(xattr_ref_cache);
> +	kmem_cache_destroy(xattr_datum_cache);
> +	kmem_cache_destroy(xattr_ref_cache);
>  #endif
>  }
>  
> -- 
> 1.9.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Sergey Senozhatsky]

Hi,

Cc Julia Lawall

On (08/04/15 13:06), Brian Norris wrote:
> On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > precede the calls are useless.
> 
> This isn't *currently* true. Only after a bit of work did I find that
> this is queued to change in the next release -- akpm has a patch:
> 
> commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Date:   Thu Jul 30 09:55:36 2015 +1000
> 
>     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> 
> Please provide the appropriate context next time. And for the moment, I will
> not apply this, because this does not work on the current MTD development tree.
> Try your patch bomb on the next release, as it's not worth my time to
> cross-merge a -next branch just for a "cleanup".
> 

Julia already has a patch set to cleanup all the existing users
(not published yet, though).

Apart from that, and this is more important, the patch in question
looks to me as 100% copy paste of a patch by Julia Lawall.

https://lkml.org/lkml/2015/6/10/31

And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".

For the record, this is the second, let's say strange, patch from Salah
that I see today.

	-ss

> > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > ---
> >  fs/jffs2/malloc.c | 27 +++++++++------------------
> >  1 file changed, 9 insertions(+), 18 deletions(-)
> > 
> > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > index b8fd651..ce11897 100644
> > --- a/fs/jffs2/malloc.c
> > +++ b/fs/jffs2/malloc.c
> > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> >  
> >  void jffs2_destroy_slab_caches(void)
> >  {
> > -	if(full_dnode_slab)
> > -		kmem_cache_destroy(full_dnode_slab);
> > -	if(raw_dirent_slab)
> > -		kmem_cache_destroy(raw_dirent_slab);
> > -	if(raw_inode_slab)
> > -		kmem_cache_destroy(raw_inode_slab);
> > -	if(tmp_dnode_info_slab)
> > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > -	if(raw_node_ref_slab)
> > -		kmem_cache_destroy(raw_node_ref_slab);
> > -	if(node_frag_slab)
> > -		kmem_cache_destroy(node_frag_slab);
> > -	if(inode_cache_slab)
> > -		kmem_cache_destroy(inode_cache_slab);
> > +	kmem_cache_destroy(full_dnode_slab);
> > +	kmem_cache_destroy(raw_dirent_slab);
> > +	kmem_cache_destroy(raw_inode_slab);
> > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > +	kmem_cache_destroy(raw_node_ref_slab);
> > +	kmem_cache_destroy(node_frag_slab);
> > +	kmem_cache_destroy(inode_cache_slab);
> >  #ifdef CONFIG_JFFS2_FS_XATTR
> > -	if (xattr_datum_cache)
> > -		kmem_cache_destroy(xattr_datum_cache);
> > -	if (xattr_ref_cache)
> > -		kmem_cache_destroy(xattr_ref_cache);
> > +	kmem_cache_destroy(xattr_datum_cache);
> > +	kmem_cache_destroy(xattr_ref_cache);
> >  #endif
> >  }
> >  
> > -- 
> > 1.9.1
> > 
> > 
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Salah Triki]

Hi,

It is not a copy paste of a cleanup patch by Julia Lawall. It is just a coincidence. In
addition, I think that, generaly, cleanups patchs are similar.

Best regards
--
Salah Triki

On Wed, Aug 05, 2015 at 10:32:47PM +0900, Sergey Senozhatsky wrote:
> 
> Hi,
> 
> Cc Julia Lawall
> 
> On (08/04/15 13:06), Brian Norris wrote:
> > On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > > precede the calls are useless.
> > 
> > This isn't *currently* true. Only after a bit of work did I find that
> > this is queued to change in the next release -- akpm has a patch:
> > 
> > commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> > Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > Date:   Thu Jul 30 09:55:36 2015 +1000
> > 
> >     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> > 
> > Please provide the appropriate context next time. And for the moment, I will
> > not apply this, because this does not work on the current MTD development tree.
> > Try your patch bomb on the next release, as it's not worth my time to
> > cross-merge a -next branch just for a "cleanup".
> > 
> 
> Julia already has a patch set to cleanup all the existing users
> (not published yet, though).
> 
> Apart from that, and this is more important, the patch in question
> looks to me as 100% copy paste of a patch by Julia Lawall.
> 
> https://lkml.org/lkml/2015/6/10/31
> 
> And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".
> 
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
> 
> 	-ss
> 
> > > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > > ---
> > >  fs/jffs2/malloc.c | 27 +++++++++------------------
> > >  1 file changed, 9 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > > index b8fd651..ce11897 100644
> > > --- a/fs/jffs2/malloc.c
> > > +++ b/fs/jffs2/malloc.c
> > > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> > >  
> > >  void jffs2_destroy_slab_caches(void)
> > >  {
> > > -	if(full_dnode_slab)
> > > -		kmem_cache_destroy(full_dnode_slab);
> > > -	if(raw_dirent_slab)
> > > -		kmem_cache_destroy(raw_dirent_slab);
> > > -	if(raw_inode_slab)
> > > -		kmem_cache_destroy(raw_inode_slab);
> > > -	if(tmp_dnode_info_slab)
> > > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > > -	if(raw_node_ref_slab)
> > > -		kmem_cache_destroy(raw_node_ref_slab);
> > > -	if(node_frag_slab)
> > > -		kmem_cache_destroy(node_frag_slab);
> > > -	if(inode_cache_slab)
> > > -		kmem_cache_destroy(inode_cache_slab);
> > > +	kmem_cache_destroy(full_dnode_slab);
> > > +	kmem_cache_destroy(raw_dirent_slab);
> > > +	kmem_cache_destroy(raw_inode_slab);
> > > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > > +	kmem_cache_destroy(raw_node_ref_slab);
> > > +	kmem_cache_destroy(node_frag_slab);
> > > +	kmem_cache_destroy(inode_cache_slab);
> > >  #ifdef CONFIG_JFFS2_FS_XATTR
> > > -	if (xattr_datum_cache)
> > > -		kmem_cache_destroy(xattr_datum_cache);
> > > -	if (xattr_ref_cache)
> > > -		kmem_cache_destroy(xattr_ref_cache);
> > > +	kmem_cache_destroy(xattr_datum_cache);
> > > +	kmem_cache_destroy(xattr_ref_cache);
> > >  #endif
> > >  }
> > >  
> > > -- 
> > > 1.9.1
> > > 
> > > 
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > 

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Salah Triki]

Hi,

It is hard to be treated as a copy-paster, especially when you're a
newcomer to the community.

Best regards
--
Salah Triki

On Wed, Aug 05, 2015 at 10:32:47PM +0900, Sergey Senozhatsky wrote:
> 
> Hi,
> 
> Cc Julia Lawall
> 
> On (08/04/15 13:06), Brian Norris wrote:
> > On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > > precede the calls are useless.
> > 
> > This isn't *currently* true. Only after a bit of work did I find that
> > this is queued to change in the next release -- akpm has a patch:
> > 
> > commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> > Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > Date:   Thu Jul 30 09:55:36 2015 +1000
> > 
> >     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> > 
> > Please provide the appropriate context next time. And for the moment, I will
> > not apply this, because this does not work on the current MTD development tree.
> > Try your patch bomb on the next release, as it's not worth my time to
> > cross-merge a -next branch just for a "cleanup".
> > 
> 
> Julia already has a patch set to cleanup all the existing users
> (not published yet, though).
> 
> Apart from that, and this is more important, the patch in question
> looks to me as 100% copy paste of a patch by Julia Lawall.
> 
> https://lkml.org/lkml/2015/6/10/31
> 
> And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".
> 
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
> 
> 	-ss
> 
> > > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > > ---
> > >  fs/jffs2/malloc.c | 27 +++++++++------------------
> > >  1 file changed, 9 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > > index b8fd651..ce11897 100644
> > > --- a/fs/jffs2/malloc.c
> > > +++ b/fs/jffs2/malloc.c
> > > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> > >  
> > >  void jffs2_destroy_slab_caches(void)
> > >  {
> > > -	if(full_dnode_slab)
> > > -		kmem_cache_destroy(full_dnode_slab);
> > > -	if(raw_dirent_slab)
> > > -		kmem_cache_destroy(raw_dirent_slab);
> > > -	if(raw_inode_slab)
> > > -		kmem_cache_destroy(raw_inode_slab);
> > > -	if(tmp_dnode_info_slab)
> > > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > > -	if(raw_node_ref_slab)
> > > -		kmem_cache_destroy(raw_node_ref_slab);
> > > -	if(node_frag_slab)
> > > -		kmem_cache_destroy(node_frag_slab);
> > > -	if(inode_cache_slab)
> > > -		kmem_cache_destroy(inode_cache_slab);
> > > +	kmem_cache_destroy(full_dnode_slab);
> > > +	kmem_cache_destroy(raw_dirent_slab);
> > > +	kmem_cache_destroy(raw_inode_slab);
> > > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > > +	kmem_cache_destroy(raw_node_ref_slab);
> > > +	kmem_cache_destroy(node_frag_slab);
> > > +	kmem_cache_destroy(inode_cache_slab);
> > >  #ifdef CONFIG_JFFS2_FS_XATTR
> > > -	if (xattr_datum_cache)
> > > -		kmem_cache_destroy(xattr_datum_cache);
> > > -	if (xattr_ref_cache)
> > > -		kmem_cache_destroy(xattr_ref_cache);
> > > +	kmem_cache_destroy(xattr_datum_cache);
> > > +	kmem_cache_destroy(xattr_ref_cache);
> > >  #endif
> > >  }
> > >  
> > > -- 
> > > 1.9.1
> > > 
> > > 
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > 

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Julia Lawall]

On Wed, 5 Aug 2015, Sergey Senozhatsky wrote:

> 
> Hi,
> 
> Cc Julia Lawall
> 
> On (08/04/15 13:06), Brian Norris wrote:
> > On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > > precede the calls are useless.
> > 
> > This isn't *currently* true. Only after a bit of work did I find that
> > this is queued to change in the next release -- akpm has a patch:
> > 
> > commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> > Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > Date:   Thu Jul 30 09:55:36 2015 +1000
> > 
> >     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> > 
> > Please provide the appropriate context next time. And for the moment, I will
> > not apply this, because this does not work on the current MTD development tree.
> > Try your patch bomb on the next release, as it's not worth my time to
> > cross-merge a -next branch just for a "cleanup".
> > 
> 
> Julia already has a patch set to cleanup all the existing users
> (not published yet, though).
> 
> Apart from that, and this is more important, the patch in question
> looks to me as 100% copy paste of a patch by Julia Lawall.
> 
> https://lkml.org/lkml/2015/6/10/31
> 
> And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".
> 
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.

I'm missing some context.  As far as I can see, kmem_cache_destroy is 
updated in linux-next.  So it seems like it could be OK to submit a patch 
to clean up the uses now.  Does Salah's patch address all three functions: 
kmem_cache_destroy, mempool_destroy, and dma_pool_destroy?

julia

> 
> 	-ss
> 
> > > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > > ---
> > >  fs/jffs2/malloc.c | 27 +++++++++------------------
> > >  1 file changed, 9 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > > index b8fd651..ce11897 100644
> > > --- a/fs/jffs2/malloc.c
> > > +++ b/fs/jffs2/malloc.c
> > > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> > >  
> > >  void jffs2_destroy_slab_caches(void)
> > >  {
> > > -	if(full_dnode_slab)
> > > -		kmem_cache_destroy(full_dnode_slab);
> > > -	if(raw_dirent_slab)
> > > -		kmem_cache_destroy(raw_dirent_slab);
> > > -	if(raw_inode_slab)
> > > -		kmem_cache_destroy(raw_inode_slab);
> > > -	if(tmp_dnode_info_slab)
> > > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > > -	if(raw_node_ref_slab)
> > > -		kmem_cache_destroy(raw_node_ref_slab);
> > > -	if(node_frag_slab)
> > > -		kmem_cache_destroy(node_frag_slab);
> > > -	if(inode_cache_slab)
> > > -		kmem_cache_destroy(inode_cache_slab);
> > > +	kmem_cache_destroy(full_dnode_slab);
> > > +	kmem_cache_destroy(raw_dirent_slab);
> > > +	kmem_cache_destroy(raw_inode_slab);
> > > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > > +	kmem_cache_destroy(raw_node_ref_slab);
> > > +	kmem_cache_destroy(node_frag_slab);
> > > +	kmem_cache_destroy(inode_cache_slab);
> > >  #ifdef CONFIG_JFFS2_FS_XATTR
> > > -	if (xattr_datum_cache)
> > > -		kmem_cache_destroy(xattr_datum_cache);
> > > -	if (xattr_ref_cache)
> > > -		kmem_cache_destroy(xattr_ref_cache);
> > > +	kmem_cache_destroy(xattr_datum_cache);
> > > +	kmem_cache_destroy(xattr_ref_cache);
> > >  #endif
> > >  }
> > >  
> > > -- 
> > > 1.9.1
> > > 
> > > 
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > 
> 

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Julia Lawall]

On Wed, 5 Aug 2015, Sergey Senozhatsky wrote:

>
> Hi,
>
> Cc Julia Lawall
>
> On (08/04/15 13:06), Brian Norris wrote:
> > On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > > precede the calls are useless.
> >
> > This isn't *currently* true. Only after a bit of work did I find that
> > this is queued to change in the next release -- akpm has a patch:
> >
> > commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> > Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > Date:   Thu Jul 30 09:55:36 2015 +1000
> >
> >     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> >
> > Please provide the appropriate context next time. And for the moment, I will
> > not apply this, because this does not work on the current MTD development tree.
> > Try your patch bomb on the next release, as it's not worth my time to
> > cross-merge a -next branch just for a "cleanup".
> >
>
> Julia already has a patch set to cleanup all the existing users
> (not published yet, though).

We should probably have put our patches into a series.  Because it is true
that the cleanup patch breaks existing code.  So one has to be aware of
the dependency between them.

julia

>
> Apart from that, and this is more important, the patch in question
> looks to me as 100% copy paste of a patch by Julia Lawall.
>
> https://lkml.org/lkml/2015/6/10/31
>
> And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".
>
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
>
> 	-ss
>
> > > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > > ---
> > >  fs/jffs2/malloc.c | 27 +++++++++------------------
> > >  1 file changed, 9 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > > index b8fd651..ce11897 100644
> > > --- a/fs/jffs2/malloc.c
> > > +++ b/fs/jffs2/malloc.c
> > > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> > >
> > >  void jffs2_destroy_slab_caches(void)
> > >  {
> > > -	if(full_dnode_slab)
> > > -		kmem_cache_destroy(full_dnode_slab);
> > > -	if(raw_dirent_slab)
> > > -		kmem_cache_destroy(raw_dirent_slab);
> > > -	if(raw_inode_slab)
> > > -		kmem_cache_destroy(raw_inode_slab);
> > > -	if(tmp_dnode_info_slab)
> > > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > > -	if(raw_node_ref_slab)
> > > -		kmem_cache_destroy(raw_node_ref_slab);
> > > -	if(node_frag_slab)
> > > -		kmem_cache_destroy(node_frag_slab);
> > > -	if(inode_cache_slab)
> > > -		kmem_cache_destroy(inode_cache_slab);
> > > +	kmem_cache_destroy(full_dnode_slab);
> > > +	kmem_cache_destroy(raw_dirent_slab);
> > > +	kmem_cache_destroy(raw_inode_slab);
> > > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > > +	kmem_cache_destroy(raw_node_ref_slab);
> > > +	kmem_cache_destroy(node_frag_slab);
> > > +	kmem_cache_destroy(inode_cache_slab);
> > >  #ifdef CONFIG_JFFS2_FS_XATTR
> > > -	if (xattr_datum_cache)
> > > -		kmem_cache_destroy(xattr_datum_cache);
> > > -	if (xattr_ref_cache)
> > > -		kmem_cache_destroy(xattr_ref_cache);
> > > +	kmem_cache_destroy(xattr_datum_cache);
> > > +	kmem_cache_destroy(xattr_ref_cache);
> > >  #endif
> > >  }
> > >
> > > --
> > > 1.9.1
> > >
> > >
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
>

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Sergey Senozhatsky]

On (08/06/15 08:16), Julia Lawall wrote:
> > On (08/04/15 13:06), Brian Norris wrote:
> > > On Tue, Aug 04, 2015 at 08:46:30PM +0100, Salah Triki wrote:
> > > > kmem_cache_destroy can be called with NULL values. Thus, the checks that
> > > > precede the calls are useless.
> > > 
> > > This isn't *currently* true. Only after a bit of work did I find that
> > > this is queued to change in the next release -- akpm has a patch:
> > > 
> > > commit 3e54c0cd3abdca0cf91854278c8633fc7df6beb1
> > > Author: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > Date:   Thu Jul 30 09:55:36 2015 +1000
> > > 
> > >     mm/slab_common: allow NULL cache pointer in kmem_cache_destroy()
> > > 
> > > Please provide the appropriate context next time. And for the moment, I will
> > > not apply this, because this does not work on the current MTD development tree.
> > > Try your patch bomb on the next release, as it's not worth my time to
> > > cross-merge a -next branch just for a "cleanup".
> > > 
> > 
> > Julia already has a patch set to cleanup all the existing users
> > (not published yet, though).
> > 
> > Apart from that, and this is more important, the patch in question
> > looks to me as 100% copy paste of a patch by Julia Lawall.
> > 
> > https://lkml.org/lkml/2015/6/10/31
> > 
> > And no, Salah, it's not "Signed-off-by: Salah Triki <salah.triki@acm.org>".
> > 
> > For the record, this is the second, let's say strange, patch from Salah
> > that I see today.
> 
> I'm missing some context.  As far as I can see, kmem_cache_destroy is 
> updated in linux-next.  So it seems like it could be OK to submit a patch 
> to clean up the uses now.  Does Salah's patch address all three functions: 
> kmem_cache_destroy, mempool_destroy, and dma_pool_destroy?
> 

no, just one that you already published.

all 3 functions are updated in -next, so you are welcome to send
your patch set.

	-ss

> > 
> > 	-ss
> > 
> > > > Signed-off-by: Salah Triki <salah.triki@acm.org>
> > > > ---
> > > >  fs/jffs2/malloc.c | 27 +++++++++------------------
> > > >  1 file changed, 9 insertions(+), 18 deletions(-)
> > > > 
> > > > diff --git a/fs/jffs2/malloc.c b/fs/jffs2/malloc.c
> > > > index b8fd651..ce11897 100644
> > > > --- a/fs/jffs2/malloc.c
> > > > +++ b/fs/jffs2/malloc.c
> > > > @@ -97,25 +97,16 @@ int __init jffs2_create_slab_caches(void)
> > > >  
> > > >  void jffs2_destroy_slab_caches(void)
> > > >  {
> > > > -	if(full_dnode_slab)
> > > > -		kmem_cache_destroy(full_dnode_slab);
> > > > -	if(raw_dirent_slab)
> > > > -		kmem_cache_destroy(raw_dirent_slab);
> > > > -	if(raw_inode_slab)
> > > > -		kmem_cache_destroy(raw_inode_slab);
> > > > -	if(tmp_dnode_info_slab)
> > > > -		kmem_cache_destroy(tmp_dnode_info_slab);
> > > > -	if(raw_node_ref_slab)
> > > > -		kmem_cache_destroy(raw_node_ref_slab);
> > > > -	if(node_frag_slab)
> > > > -		kmem_cache_destroy(node_frag_slab);
> > > > -	if(inode_cache_slab)
> > > > -		kmem_cache_destroy(inode_cache_slab);
> > > > +	kmem_cache_destroy(full_dnode_slab);
> > > > +	kmem_cache_destroy(raw_dirent_slab);
> > > > +	kmem_cache_destroy(raw_inode_slab);
> > > > +	kmem_cache_destroy(tmp_dnode_info_slab);
> > > > +	kmem_cache_destroy(raw_node_ref_slab);
> > > > +	kmem_cache_destroy(node_frag_slab);
> > > > +	kmem_cache_destroy(inode_cache_slab);
> > > >  #ifdef CONFIG_JFFS2_FS_XATTR
> > > > -	if (xattr_datum_cache)
> > > > -		kmem_cache_destroy(xattr_datum_cache);
> > > > -	if (xattr_ref_cache)
> > > > -		kmem_cache_destroy(xattr_ref_cache);
> > > > +	kmem_cache_destroy(xattr_datum_cache);
> > > > +	kmem_cache_destroy(xattr_ref_cache);
> > > >  #endif
> > > >  }
> > > >  
> > > > -- 
> > > > 1.9.1
> > > > 
> > > > 
> > > > ______________________________________________________
> > > > Linux MTD discussion mailing list
> > > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> > > 
> > 
> 

Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

[Sergey Senozhatsky]

On (08/06/15 09:24), Julia Lawall wrote:
> > >
> > > Please provide the appropriate context next time. And for the moment, I will
> > > not apply this, because this does not work on the current MTD development tree.
> > > Try your patch bomb on the next release, as it's not worth my time to
> > > cross-merge a -next branch just for a "cleanup".
> > >
> >
> > Julia already has a patch set to cleanup all the existing users
> > (not published yet, though).
> 
> We should probably have put our patches into a series.  Because it is true
> that the cleanup patch breaks existing code.  So one has to be aware of
> the dependency between them.
> 

I guess we can wait until 3 patches in question will land in Linus's
tree. What do you think?

	-ss