[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230523010627.GD11598@frogsfrogsfrogs>
Date: Mon, 22 May 2023 18:06:27 -0700
From: "Darrick J. Wong" <djwong@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Matthew Wilcox <willy@...radead.org>, Jens Axboe <axboe@...nel.dk>,
Xiubo Li <xiubli@...hat.com>,
Ilya Dryomov <idryomov@...il.com>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>,
Theodore Ts'o <tytso@....edu>,
Jaegeuk Kim <jaegeuk@...nel.org>, Chao Yu <chao@...nel.org>,
Miklos Szeredi <miklos@...redi.hu>,
Andreas Gruenbacher <agruenba@...hat.com>,
Trond Myklebust <trond.myklebust@...merspace.com>,
Anna Schumaker <anna@...nel.org>,
Damien Le Moal <dlemoal@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-block@...r.kernel.org, ceph-devel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-ext4@...r.kernel.org,
"open list:F2FS FILE SYSTEM" <linux-f2fs-devel@...ts.sourceforge.net>,
cluster-devel@...hat.com, linux-xfs@...r.kernel.org,
linux-nfs@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 08/13] iomap: assign current->backing_dev_info in
iomap_file_buffered_write
On Fri, May 19, 2023 at 11:35:16AM +0200, Christoph Hellwig wrote:
> Move the assignment to current->backing_dev_info from the callers into
> iomap_file_buffered_write to reduce boiler plate code and reduce the
> scope to just around the page dirtying loop.
>
> Note that zonefs was missing this assignment before.
I'm still wondering (a) what the hell current->backing_dev_info is for,
and (b) if we need it around the iomap_unshare operation.
$ git grep current..backing_dev_info
fs/btrfs/file.c:1148: current->backing_dev_info = inode_to_bdi(inode);
fs/btrfs/file.c:1169: current->backing_dev_info = NULL;
fs/btrfs/file.c:1692: current->backing_dev_info = NULL;
fs/ceph/file.c:1795: current->backing_dev_info = inode_to_bdi(inode);
fs/ceph/file.c:1943: current->backing_dev_info = NULL;
fs/ext4/file.c:288: current->backing_dev_info = inode_to_bdi(inode);
fs/ext4/file.c:290: current->backing_dev_info = NULL;
fs/f2fs/file.c:4520: current->backing_dev_info = inode_to_bdi(inode);
fs/f2fs/file.c:4522: current->backing_dev_info = NULL;
fs/fuse/file.c:1366: current->backing_dev_info = inode_to_bdi(inode);
fs/fuse/file.c:1412: current->backing_dev_info = NULL;
fs/gfs2/file.c:1044: current->backing_dev_info = inode_to_bdi(inode);
fs/gfs2/file.c:1048: current->backing_dev_info = NULL;
fs/nfs/file.c:652: current->backing_dev_info = inode_to_bdi(inode);
fs/nfs/file.c:654: current->backing_dev_info = NULL;
fs/ntfs/file.c:1914: current->backing_dev_info = inode_to_bdi(vi);
fs/ntfs/file.c:1918: current->backing_dev_info = NULL;
fs/ntfs3/file.c:823: current->backing_dev_info = inode_to_bdi(inode);
fs/ntfs3/file.c:996: current->backing_dev_info = NULL;
fs/xfs/xfs_file.c:721: current->backing_dev_info = inode_to_bdi(inode);
fs/xfs/xfs_file.c:756: current->backing_dev_info = NULL;
mm/filemap.c:3995: current->backing_dev_info = inode_to_bdi(inode);
mm/filemap.c:4056: current->backing_dev_info = NULL;
AFAICT nobody uses it at all? Unless there's some bizarre user that
isn't extracting it from @current?
Oh, hey, new question (c) isn't this set incorrectly for xfs realtime
files?
--D
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
> fs/gfs2/file.c | 3 ---
> fs/iomap/buffered-io.c | 3 +++
> fs/xfs/xfs_file.c | 5 -----
> 3 files changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 499ef174dec138..261897fcfbc495 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -25,7 +25,6 @@
> #include <linux/dlm.h>
> #include <linux/dlm_plock.h>
> #include <linux/delay.h>
> -#include <linux/backing-dev.h>
> #include <linux/fileattr.h>
>
> #include "gfs2.h"
> @@ -1041,11 +1040,9 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb,
> goto out_unlock;
> }
>
> - current->backing_dev_info = inode_to_bdi(inode);
> pagefault_disable();
> ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops);
> pagefault_enable();
> - current->backing_dev_info = NULL;
> if (ret > 0)
> written += ret;
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 550525a525c45c..b2779bd1f10611 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -3,6 +3,7 @@
> * Copyright (C) 2010 Red Hat, Inc.
> * Copyright (C) 2016-2019 Christoph Hellwig.
> */
> +#include <linux/backing-dev.h>
> #include <linux/module.h>
> #include <linux/compiler.h>
> #include <linux/fs.h>
> @@ -869,8 +870,10 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i,
> if (iocb->ki_flags & IOCB_NOWAIT)
> iter.flags |= IOMAP_NOWAIT;
>
> + current->backing_dev_info = inode_to_bdi(iter.inode);
> while ((ret = iomap_iter(&iter, ops)) > 0)
> iter.processed = iomap_write_iter(&iter, i);
> + current->backing_dev_info = NULL;
>
> if (unlikely(ret < 0))
> return ret;
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index bfba10e0b0f3c2..98d763cc3b114c 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -27,7 +27,6 @@
>
> #include <linux/dax.h>
> #include <linux/falloc.h>
> -#include <linux/backing-dev.h>
> #include <linux/mman.h>
> #include <linux/fadvise.h>
> #include <linux/mount.h>
> @@ -717,9 +716,6 @@ xfs_file_buffered_write(
> if (ret)
> goto out;
>
> - /* We can write back this queue in page reclaim */
> - current->backing_dev_info = inode_to_bdi(inode);
> -
> trace_xfs_file_buffered_write(iocb, from);
> ret = iomap_file_buffered_write(iocb, from,
> &xfs_buffered_write_iomap_ops);
> @@ -751,7 +747,6 @@ xfs_file_buffered_write(
> goto write_retry;
> }
>
> - current->backing_dev_info = NULL;
> out:
> if (iolock)
> xfs_iunlock(ip, iolock);
> --
> 2.39.2
>
Powered by blists - more mailing lists