btrfs: use GFP_NOFS instead of GFP_KERNEL

PATCH In review Miao Xie 2 participants 3 messages Mar 24, 2011 – Mar 25, 2011 Btrfs file system

Share & tools

Miao Xie

author

2011-03-24 11:41 UTC #0 lore raw

PATCH v5 1/2 btrfs: use GFP_NOFS instead of GFP_KERNEL

In the filesystem context, we must allocate memory by GFP_NOFS, or we may start another filesystem operation and make kswap thread hang up.

Signed-off-by Miao Xie <[email protected]>

---
 fs/btrfs/extent-tree.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

fs/btrfs/extent-tree.c +2 -2

index f1db57d..42061d2 100644
@@ -471,7 +471,7 @@ static int cache_block_group(struct btrfs_block_group_cache *cache,
 	if (load_cache_only)
 		return 0;
 
-	caching_ctl = kzalloc(sizeof(*caching_ctl), GFP_KERNEL);
+	caching_ctl = kzalloc(sizeof(*caching_ctl), GFP_NOFS);
 	BUG_ON(!caching_ctl);
 
 	INIT_LIST_HEAD(&caching_ctl->list);

@@ -1743,7 +1743,7 @@ static int remove_extent_backref(struct btrfs_trans_handle *trans,
 static void btrfs_issue_discard(struct block_device *bdev,
 				u64 start, u64 len)
 {
-	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_KERNEL,
+	blkdev_issue_discard(bdev, start >> 9, len >> 9, GFP_NOFS,
 			BLKDEV_IFL_WAIT | BLKDEV_IFL_BARRIER);
 }

Signature · 1 line

1.7.3.1

Miao Xie

author

2011-03-25 01:56 UTC #2 lore raw

Quoted 6 lines · David Sterba

Quoted 2 lines · Miao Xie
In the filesystem context, we must allocate memory by GFP_NOFS, or we may start another filesystem operation and make kswap thread hang up.
indeed. Did you check for other GFP_KERNEL allocations? I've found 8 more them and at least these look like candidates for GFP_NOFS too:

I just fix the ones, which should use GFP_NOFS.

I think not all of the GFP_KERNEL allocations are wrong, if we don't hold any btrfs's lock except relative inode's i_mutex and not in the context of the transaction, we can use GFP_KERNEL. So the following GFP_KERNEL allocations are right, I think.

Thanks
Miao

Quoted 66 lines

fs/btrfs/acl.c +1 -1

index de34bfa..76b9218 100644
@@ -289,7 +289,7 @@ int btrfs_acl_chmod(struct inode *inode)
        if (IS_ERR(acl) || !acl)
                return PTR_ERR(acl);
 
-       clone = posix_acl_clone(acl, GFP_KERNEL);
+       clone = posix_acl_clone(acl, GFP_NOFS);
        posix_acl_release(acl);
        if (!clone)
                return -ENOMEM;

fs/btrfs/file.c +1 -1

index f447b78..eb5c01d 100644
@@ -998,7 +998,7 @@ static ssize_t btrfs_file_aio_write(struct kiocb *iocb,
        nrptrs = min((iov_iter_count(&i) + PAGE_CACHE_SIZE - 1) /
                     PAGE_CACHE_SIZE, PAGE_CACHE_SIZE /
                     (sizeof(struct page *)));
-       pages = kmalloc(nrptrs * sizeof(struct page *), GFP_KERNEL);
+       pages = kmalloc(nrptrs * sizeof(struct page *), GFP_NOFS);
        if (!pages) {
                ret = -ENOMEM;
                goto out;

fs/btrfs/ioctl.c +1 -1

index d1bace3..e9b9648 100644
@@ -1644,7 +1644,7 @@ static int btrfs_ioctl_defrag(struct file *file, void __user *argp)
                        goto out;
                }
 
-               range = kzalloc(sizeof(*range), GFP_KERNEL);
+               range = kzalloc(sizeof(*range), GFP_NOFS);
                if (!range) {
                        ret = -ENOMEM;
                        goto out;

fs/btrfs/super.c +2 -2

index d39a989..5e0fff7 100644
@@ -399,7 +399,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
         * strsep changes the string, duplicate it because parse_options
         * gets called twice
         */
-       opts = kstrdup(options, GFP_KERNEL);
+       opts = kstrdup(options, GFP_NOFS);
        if (!opts)
                return -ENOMEM;
        orig = opts;

@@ -446,7 +446,7 @@ static int btrfs_parse_early_options(const char *options, fmode_t flags,
         * mount path doesn't care if it's the default volume or another one.
         */
        if (!*subvol_name) {
-               *subvol_name = kstrdup(".", GFP_KERNEL);
+               *subvol_name = kstrdup(".", GFP_NOFS);
                if (!*subvol_name)
                        return -ENOMEM;
        }

dave