Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200251 > unrolled thread

[PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

Started bySalah Triki <salah.triki@acm.org>
First post2015-08-04 21:50 +0200
Last post2015-08-06 16:30 +0200
Articles 10 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Salah Triki <salah.triki@acm.org> - 2015-08-04 21:50 +0200
    Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Richard Weinberger <richard.weinberger@gmail.com> - 2015-08-04 22:10 +0200
    Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Brian Norris <computersforpeace@gmail.com> - 2015-08-04 22:10 +0200
      Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-08-05 15:40 +0200
        Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Salah Triki <salah.triki@acm.org> - 2015-08-06 00:20 +0200
        Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Salah Triki <salah.triki@acm.org> - 2015-08-06 01:20 +0200
        Re: [PATCH] jffs2: Remove NULL checks from  jffs2_destroy_slab_caches Julia Lawall <julia.lawall@lip6.fr> - 2015-08-06 08:20 +0200
          Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-08-06 16:20 +0200
        Re: [PATCH] jffs2: Remove NULL checks from  jffs2_destroy_slab_caches Julia Lawall <julia.lawall@lip6.fr> - 2015-08-06 09:30 +0200
          Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2015-08-06 16:30 +0200

#1200251 — [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

FromSalah Triki <salah.triki@acm.org>
Date2015-08-04 21:50 +0200
Subject[PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches
Message-ID<pTQnN-3rL-17@gated-at.bofh.it>
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1200265

FromRichard Weinberger <richard.weinberger@gmail.com>
Date2015-08-04 22:10 +0200
Message-ID<pTQH8-45x-21@gated-at.bofh.it>
In reply to#1200251
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200267

FromBrian Norris <computersforpeace@gmail.com>
Date2015-08-04 22:10 +0200
Message-ID<pTQH8-45x-5@gated-at.bofh.it>
In reply to#1200251
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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200754

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2015-08-05 15:40 +0200
Message-ID<pU75h-2L3-29@gated-at.bofh.it>
In reply to#1200267
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/
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201281

FromSalah Triki <salah.triki@acm.org>
Date2015-08-06 00:20 +0200
Message-ID<pUfcw-69Y-73@gated-at.bofh.it>
In reply to#1200754
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/
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201333

FromSalah Triki <salah.triki@acm.org>
Date2015-08-06 01:20 +0200
Message-ID<pUg8y-7vP-15@gated-at.bofh.it>
In reply to#1200754
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/
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201459 — Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-08-06 08:20 +0200
SubjectRe: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches
Message-ID<pUmGZ-sF-9@gated-at.bofh.it>
In reply to#1200754

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/
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201778

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2015-08-06 16:20 +0200
Message-ID<pUubw-2Ur-13@gated-at.bofh.it>
In reply to#1201459
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/
> > > 
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201515 — Re: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches

FromJulia Lawall <julia.lawall@lip6.fr>
Date2015-08-06 09:30 +0200
SubjectRe: [PATCH] jffs2: Remove NULL checks from jffs2_destroy_slab_caches
Message-ID<pUnMJ-1Zx-3@gated-at.bofh.it>
In reply to#1200754

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/
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201797

FromSergey Senozhatsky <sergey.senozhatsky@gmail.com>
Date2015-08-06 16:30 +0200
Message-ID<pUuld-367-37@gated-at.bofh.it>
In reply to#1201515
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web