kmem_cache_destroy can be called with NULL values. Thus, the checks that
precede the calls are useless.
Signed-off-by: Salah Triki <[email protected]>
---
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
On Tue, Aug 4, 2015 at 9:46 PM, Salah Triki <[email protected]> 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
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 <[email protected]>
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 <[email protected]>
> ---
> 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/
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 <[email protected]>
> 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 <[email protected]>".
For the record, this is the second, let's say strange, patch from Salah
that I see today.
-ss
> > Signed-off-by: Salah Triki <[email protected]>
> > ---
> > 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/
>
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 <[email protected]>
> > 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 <[email protected]>".
>
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
>
> -ss
>
> > > Signed-off-by: Salah Triki <[email protected]>
> > > ---
> > > 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/
> >
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 <[email protected]>
> > 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 <[email protected]>".
>
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
>
> -ss
>
> > > Signed-off-by: Salah Triki <[email protected]>
> > > ---
> > > 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/
> >
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 <[email protected]>
> > 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 <[email protected]>".
>
> 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 <[email protected]>
> > > ---
> > > 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/
> >
>
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 <[email protected]>
> > 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 <[email protected]>".
>
> For the record, this is the second, let's say strange, patch from Salah
> that I see today.
>
> -ss
>
> > > Signed-off-by: Salah Triki <[email protected]>
> > > ---
> > > 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/
> >
>
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 <[email protected]>
> > > 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 <[email protected]>".
> >
> > 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 <[email protected]>
> > > > ---
> > > > 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/
> > >
> >
>
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