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-ext4" 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-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
