Re: [PATCH 10/22] btrfs: alloc space cache inode with GFP_NOFS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 19, 2018 at 06:35:33PM +0300, Nikolay Borisov wrote:
> 
> 
> On 19.07.2018 17:49, Josef Bacik wrote:
> > If we're allocating a new space cache inode it's likely going to be
> > under a transaction handle, so we need to use GFP_NOFS to keep from
> > deadlocking.  Otherwise GFP_KERNEL is fine.
> > 
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > ---
> >  fs/btrfs/free-space-cache.c | 5 +++++
> >  fs/btrfs/inode.c            | 3 ++-
> >  fs/btrfs/transaction.h      | 1 +
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> > index d5f80cb300be..13bc514e4e16 100644
> > --- a/fs/btrfs/free-space-cache.c
> > +++ b/fs/btrfs/free-space-cache.c
> > @@ -68,7 +68,12 @@ static struct inode *__lookup_free_space_inode(struct btrfs_root *root,
> >  	btrfs_disk_key_to_cpu(&location, &disk_key);
> >  	btrfs_release_path(path);
> >  
> > +	/* We need this set so that we use GFP_NOFS when allocating our inode. */
> > +	if (current->journal_info == NULL)
> > +		current->journal_info = BTRFS_TRANS_STUB;
> >  	inode = btrfs_iget(fs_info->sb, &location, root, NULL);
> > +	if (current->journal_info == BTRFS_TRANS_STUB)
> > +		current->journal_info = NULL;
> This is not safe in the face of stacked filesystem, i.e ext4 uses the
> journal_info.
> 
> >  	if (IS_ERR(inode))
> >  		return inode;
> >  	if (is_bad_inode(inode)) {
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index eba61bcb9bb3..14ecfe5d6110 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -9211,8 +9211,9 @@ struct inode *btrfs_alloc_inode(struct super_block *sb)
> >  	struct btrfs_fs_info *fs_info = btrfs_sb(sb);
> >  	struct btrfs_inode *ei;
> >  	struct inode *inode;
> > +	gfp_t flags = (current->journal_info) ? GFP_NOFS : GFP_KERNEL;
> >  
> > -	ei = kmem_cache_alloc(btrfs_inode_cachep, GFP_KERNEL);
> > +	ei = kmem_cache_alloc(btrfs_inode_cachep, flags);
> 
> Why don't you just hardcode GFP_NOFS? We should be striving at removing
> abuse of ->journal_info than proliferating it.

We're also removing unnecessary use of GFP_NOFS. The right way here is
to use memalloc_nofs_save/memalloc_nofs_restore around btrfs_iget.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux