Re: [PATCH] JBD: use GFP_NOFS in kmalloc
On Sep 19, 2007 12:22 -0700, Mingming Cao wrote: > Convert the GFP_KERNEL flag used in JBD/JBD2 to GFP_NOFS, consistent > with the rest of kmalloc flag used in the JBD/JBD2 layer. > > @@ -653,7 +653,7 @@ static journal_t * journal_init_common ( > - journal = kmalloc(sizeof(*journal), GFP_KERNEL); > + journal = kmalloc(sizeof(*journal), GFP_NOFS); > @@ -723,7 +723,7 @@ journal_t * journal_init_dev(struct bloc > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); > @@ -777,7 +777,7 @@ journal_t * journal_init_inode (struct i > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); Is there a reason for this change except "it's in a filesystem, so it should be GFP_NOFS"? We are only doing journal setup during mount so there shouldn't be any problem using GFP_KERNEL. I don't think it will inject any defect into the code, but I don't think it is needed either. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] JBD: use GFP_NOFS in kmalloc
On Wed, 2007-09-19 at 14:34 -0700, Andrew Morton wrote: > On Wed, 19 Sep 2007 12:22:09 -0700 > Mingming Cao <[EMAIL PROTECTED]> wrote: > > > Convert the GFP_KERNEL flag used in JBD/JBD2 to GFP_NOFS, consistent > > with the rest of kmalloc flag used in the JBD/JBD2 layer. > > > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> > > > > --- > > fs/jbd/journal.c |6 +++--- > > fs/jbd/revoke.c |8 > > fs/jbd2/journal.c |6 +++--- > > fs/jbd2/revoke.c |8 > > 4 files changed, 14 insertions(+), 14 deletions(-) > > > > Index: linux-2.6.23-rc6/fs/jbd/journal.c > > === > > --- linux-2.6.23-rc6.orig/fs/jbd/journal.c 2007-09-19 11:51:10.0 > > -0700 > > +++ linux-2.6.23-rc6/fs/jbd/journal.c 2007-09-19 11:51:57.0 > > -0700 > > @@ -653,7 +653,7 @@ static journal_t * journal_init_common ( > > journal_t *journal; > > int err; > > > > - journal = kmalloc(sizeof(*journal), GFP_KERNEL); > > + journal = kmalloc(sizeof(*journal), GFP_NOFS); > > if (!journal) > > goto fail; > > memset(journal, 0, sizeof(*journal)); > > @@ -723,7 +723,7 @@ journal_t * journal_init_dev(struct bloc > > journal->j_blocksize = blocksize; > > n = journal->j_blocksize / sizeof(journal_block_tag_t); > > journal->j_wbufsize = n; > > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); > > if (!journal->j_wbuf) { > > printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", > > __FUNCTION__); > > @@ -777,7 +777,7 @@ journal_t * journal_init_inode (struct i > > /* journal descriptor can store up to n blocks -bzzz */ > > n = journal->j_blocksize / sizeof(journal_block_tag_t); > > journal->j_wbufsize = n; > > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); > > if (!journal->j_wbuf) { > > printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", > > __FUNCTION__); > > Index: linux-2.6.23-rc6/fs/jbd/revoke.c > > === > > --- linux-2.6.23-rc6.orig/fs/jbd/revoke.c 2007-09-19 11:51:30.0 > > -0700 > > +++ linux-2.6.23-rc6/fs/jbd/revoke.c2007-09-19 11:52:34.0 > > -0700 > > @@ -206,7 +206,7 @@ int journal_init_revoke(journal_t *journ > > while((tmp >>= 1UL) != 0UL) > > shift++; > > > > - journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, > > GFP_KERNEL); > > + journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, > > GFP_NOFS); > > if (!journal->j_revoke_table[0]) > > return -ENOMEM; > > journal->j_revoke = journal->j_revoke_table[0]; > > @@ -219,7 +219,7 @@ int journal_init_revoke(journal_t *journ > > journal->j_revoke->hash_shift = shift; > > > > journal->j_revoke->hash_table = > > - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); > > + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); > > if (!journal->j_revoke->hash_table) { > > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); > > journal->j_revoke = NULL; > > @@ -229,7 +229,7 @@ int journal_init_revoke(journal_t *journ > > for (tmp = 0; tmp < hash_size; tmp++) > > INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]); > > > > - journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, > > GFP_KERNEL); > > + journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, > > GFP_NOFS); > > if (!journal->j_revoke_table[1]) { > > kfree(journal->j_revoke_table[0]->hash_table); > > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); > > @@ -246,7 +246,7 @@ int journal_init_revoke(journal_t *journ > > journal->j_revoke->hash_shift = shift; > > > > journal->j_revoke->hash_table = > > - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); > > + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); > > if (!journal->j_revoke->hash_table) { > > kfree(journal->j_revoke_table[0]->hash_table); > > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); > > These were all OK using GFP_KERNEL. > > GFP_NOFS should only be used when the caller is holding some fs locks which > might cause a deadlock if that caller reentered the fs in ->writepage (and > maybe put_inode and such). That isn't the case in any of the above code, > which is all mount time stuff (I think). > You are right they are all occur at initialization time. > ext3/4 should be using GFP_NOFS when the caller has a transaction open, has > a page locked, is holding i_mutex, etc. > Thanks for your
Re: [PATCH] JBD: use GFP_NOFS in kmalloc
On Wed, 19 Sep 2007 12:22:09 -0700 Mingming Cao <[EMAIL PROTECTED]> wrote: > Convert the GFP_KERNEL flag used in JBD/JBD2 to GFP_NOFS, consistent > with the rest of kmalloc flag used in the JBD/JBD2 layer. > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> > > --- > fs/jbd/journal.c |6 +++--- > fs/jbd/revoke.c |8 > fs/jbd2/journal.c |6 +++--- > fs/jbd2/revoke.c |8 > 4 files changed, 14 insertions(+), 14 deletions(-) > > Index: linux-2.6.23-rc6/fs/jbd/journal.c > === > --- linux-2.6.23-rc6.orig/fs/jbd/journal.c2007-09-19 11:51:10.0 > -0700 > +++ linux-2.6.23-rc6/fs/jbd/journal.c 2007-09-19 11:51:57.0 -0700 > @@ -653,7 +653,7 @@ static journal_t * journal_init_common ( > journal_t *journal; > int err; > > - journal = kmalloc(sizeof(*journal), GFP_KERNEL); > + journal = kmalloc(sizeof(*journal), GFP_NOFS); > if (!journal) > goto fail; > memset(journal, 0, sizeof(*journal)); > @@ -723,7 +723,7 @@ journal_t * journal_init_dev(struct bloc > journal->j_blocksize = blocksize; > n = journal->j_blocksize / sizeof(journal_block_tag_t); > journal->j_wbufsize = n; > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); > if (!journal->j_wbuf) { > printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", > __FUNCTION__); > @@ -777,7 +777,7 @@ journal_t * journal_init_inode (struct i > /* journal descriptor can store up to n blocks -bzzz */ > n = journal->j_blocksize / sizeof(journal_block_tag_t); > journal->j_wbufsize = n; > - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); > + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); > if (!journal->j_wbuf) { > printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", > __FUNCTION__); > Index: linux-2.6.23-rc6/fs/jbd/revoke.c > === > --- linux-2.6.23-rc6.orig/fs/jbd/revoke.c 2007-09-19 11:51:30.0 > -0700 > +++ linux-2.6.23-rc6/fs/jbd/revoke.c 2007-09-19 11:52:34.0 -0700 > @@ -206,7 +206,7 @@ int journal_init_revoke(journal_t *journ > while((tmp >>= 1UL) != 0UL) > shift++; > > - journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, > GFP_KERNEL); > + journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, > GFP_NOFS); > if (!journal->j_revoke_table[0]) > return -ENOMEM; > journal->j_revoke = journal->j_revoke_table[0]; > @@ -219,7 +219,7 @@ int journal_init_revoke(journal_t *journ > journal->j_revoke->hash_shift = shift; > > journal->j_revoke->hash_table = > - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); > + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); > if (!journal->j_revoke->hash_table) { > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); > journal->j_revoke = NULL; > @@ -229,7 +229,7 @@ int journal_init_revoke(journal_t *journ > for (tmp = 0; tmp < hash_size; tmp++) > INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]); > > - journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, > GFP_KERNEL); > + journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, > GFP_NOFS); > if (!journal->j_revoke_table[1]) { > kfree(journal->j_revoke_table[0]->hash_table); > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); > @@ -246,7 +246,7 @@ int journal_init_revoke(journal_t *journ > journal->j_revoke->hash_shift = shift; > > journal->j_revoke->hash_table = > - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); > + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); > if (!journal->j_revoke->hash_table) { > kfree(journal->j_revoke_table[0]->hash_table); > kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); These were all OK using GFP_KERNEL. GFP_NOFS should only be used when the caller is holding some fs locks which might cause a deadlock if that caller reentered the fs in ->writepage (and maybe put_inode and such). That isn't the case in any of the above code, which is all mount time stuff (I think). ext3/4 should be using GFP_NOFS when the caller has a transaction open, has a page locked, is holding i_mutex, etc. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] JBD: use GFP_NOFS in kmalloc
Convert the GFP_KERNEL flag used in JBD/JBD2 to GFP_NOFS, consistent with the rest of kmalloc flag used in the JBD/JBD2 layer. Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> --- fs/jbd/journal.c |6 +++--- fs/jbd/revoke.c |8 fs/jbd2/journal.c |6 +++--- fs/jbd2/revoke.c |8 4 files changed, 14 insertions(+), 14 deletions(-) Index: linux-2.6.23-rc6/fs/jbd/journal.c === --- linux-2.6.23-rc6.orig/fs/jbd/journal.c 2007-09-19 11:51:10.0 -0700 +++ linux-2.6.23-rc6/fs/jbd/journal.c 2007-09-19 11:51:57.0 -0700 @@ -653,7 +653,7 @@ static journal_t * journal_init_common ( journal_t *journal; int err; - journal = kmalloc(sizeof(*journal), GFP_KERNEL); + journal = kmalloc(sizeof(*journal), GFP_NOFS); if (!journal) goto fail; memset(journal, 0, sizeof(*journal)); @@ -723,7 +723,7 @@ journal_t * journal_init_dev(struct bloc journal->j_blocksize = blocksize; n = journal->j_blocksize / sizeof(journal_block_tag_t); journal->j_wbufsize = n; - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); if (!journal->j_wbuf) { printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", __FUNCTION__); @@ -777,7 +777,7 @@ journal_t * journal_init_inode (struct i /* journal descriptor can store up to n blocks -bzzz */ n = journal->j_blocksize / sizeof(journal_block_tag_t); journal->j_wbufsize = n; - journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_KERNEL); + journal->j_wbuf = kmalloc(n * sizeof(struct buffer_head*), GFP_NOFS); if (!journal->j_wbuf) { printk(KERN_ERR "%s: Cant allocate bhs for commit thread\n", __FUNCTION__); Index: linux-2.6.23-rc6/fs/jbd/revoke.c === --- linux-2.6.23-rc6.orig/fs/jbd/revoke.c 2007-09-19 11:51:30.0 -0700 +++ linux-2.6.23-rc6/fs/jbd/revoke.c2007-09-19 11:52:34.0 -0700 @@ -206,7 +206,7 @@ int journal_init_revoke(journal_t *journ while((tmp >>= 1UL) != 0UL) shift++; - journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, GFP_KERNEL); + journal->j_revoke_table[0] = kmem_cache_alloc(revoke_table_cache, GFP_NOFS); if (!journal->j_revoke_table[0]) return -ENOMEM; journal->j_revoke = journal->j_revoke_table[0]; @@ -219,7 +219,7 @@ int journal_init_revoke(journal_t *journ journal->j_revoke->hash_shift = shift; journal->j_revoke->hash_table = - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); if (!journal->j_revoke->hash_table) { kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); journal->j_revoke = NULL; @@ -229,7 +229,7 @@ int journal_init_revoke(journal_t *journ for (tmp = 0; tmp < hash_size; tmp++) INIT_LIST_HEAD(&journal->j_revoke->hash_table[tmp]); - journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, GFP_KERNEL); + journal->j_revoke_table[1] = kmem_cache_alloc(revoke_table_cache, GFP_NOFS); if (!journal->j_revoke_table[1]) { kfree(journal->j_revoke_table[0]->hash_table); kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); @@ -246,7 +246,7 @@ int journal_init_revoke(journal_t *journ journal->j_revoke->hash_shift = shift; journal->j_revoke->hash_table = - kmalloc(hash_size * sizeof(struct list_head), GFP_KERNEL); + kmalloc(hash_size * sizeof(struct list_head), GFP_NOFS); if (!journal->j_revoke->hash_table) { kfree(journal->j_revoke_table[0]->hash_table); kmem_cache_free(revoke_table_cache, journal->j_revoke_table[0]); Index: linux-2.6.23-rc6/fs/jbd2/journal.c === --- linux-2.6.23-rc6.orig/fs/jbd2/journal.c 2007-09-19 11:52:48.0 -0700 +++ linux-2.6.23-rc6/fs/jbd2/journal.c 2007-09-19 11:53:12.0 -0700 @@ -654,7 +654,7 @@ static journal_t * journal_init_common ( journal_t *journal; int err; - journal = kmalloc(sizeof(*journal), GFP_KERNEL); + journal = kmalloc(sizeof(*journal), GFP_NOFS); if (!journal) goto fail; memset(journal, 0, sizeof(*journal)); @@ -724,7 +724,7 @@ journal_t * jbd2_journal_init_dev(struct journal->j_blocksize = blocksize; n = journal->j_blocksize / sizeof(journal_block_tag_t); journal->j_wbufsize = n; - journal->j_wbuf = kmalloc(