Hi,
Al, here's some cleanup that you mentioned back in December that I got
around to (https://lkml.org/lkml/2014/12/15/28).
In summary, the rw parameter to a_ops->direct_IO() is redundant with
.type in struct iov_iter. Additionally, rw is inconsistently checked for
being a WRITE; some filesystems do rw == WRITE, others do rw & WRITE,
and others do both within the same function :) The distinction is that
swapout may OR in the ITER_BVEC flag in the rw passed to ->direct_IO(),
so the two are not equivalent (although this really only happens for
swap-over-NFS, but it's scary nonetheless). After looking through all of
these, it definitely looks like every check means for ANY write, not
just non-kernel writes.
So, the solution presented here is:
- Add a helper, iov_iter_rw(), which always returns either READ or
WRITE, no ITER_.* or REQ_.* nonsense mixed in. For consistency, the
return value is always checked for equality
- Get rid of all uses of rw in any implementations of direct_IO,
starting with the generic code
- Nuke the actual parameter and update the documentation
I decided to squish all of the filesystems together in patch 4 to avoid
inundating the mailing lists with 20+ mostly two-line patches, but I can
split those out if that's any better. Additionally, patch 1 pulls fs.h
into uio.h, which seems undesirable.
These were mostly just compile tested, with a couple of direct I/O
xfstests run on btrfs as quick sanity check, so getting some more eyes
on is probably a good thing. They should apply on top of v4.0-rc4.
Please comment away.
Thank you,
Omar Sandoval (5):
new helper: iov_iter_rw()
Remove rw from {,__,do_}blockdev_direct_IO()
Remove rw from dax_{do_,}io()
direct_IO: use iov_iter_rw() instead of rw everywhere
direct_IO: remove rw from a_ops->direct_IO()
Documentation/filesystems/Locking | 2 +-
Documentation/filesystems/vfs.txt | 2 +-
drivers/staging/lustre/lustre/llite/rw26.c | 22 ++++++++---------
fs/9p/vfs_addr.c | 2 +-
fs/affs/file.c | 9 +++----
fs/block_dev.c | 8 +++---
fs/btrfs/inode.c | 24 +++++++++---------
fs/ceph/addr.c | 3 +--
fs/cifs/file.c | 3 +--
fs/dax.c | 27 ++++++++++-----------
fs/direct-io.c | 39 ++++++++++++++----------------
fs/exofs/inode.c | 4 +--
fs/ext2/inode.c | 11 ++++-----
fs/ext3/inode.c | 14 +++++------
fs/ext4/ext4.h | 4 +--
fs/ext4/indirect.c | 25 ++++++++++---------
fs/ext4/inode.c | 28 ++++++++++-----------
fs/f2fs/data.c | 22 ++++++++---------
fs/fat/inode.c | 9 +++----
fs/fuse/file.c | 16 ++++++------
fs/gfs2/aops.c | 16 ++++++------
fs/hfs/inode.c | 8 +++---
fs/hfsplus/inode.c | 9 +++----
fs/jfs/inode.c | 8 +++---
fs/nfs/direct.c | 4 +--
fs/nilfs2/inode.c | 10 +++-----
fs/ocfs2/aops.c | 22 +++++++----------
fs/reiserfs/inode.c | 8 +++---
fs/udf/file.c | 3 +--
fs/udf/inode.c | 7 +++---
fs/xfs/xfs_aops.c | 12 ++++-----
include/linux/fs.h | 24 +++++++++---------
include/linux/nfs_fs.h | 2 +-
include/linux/uio.h | 10 ++++++++
mm/filemap.c | 4 +--
mm/page_io.c | 4 +--
36 files changed, 206 insertions(+), 219 deletions(-)
--
2.3.3
Get either READ or WRITE out of iter->type.
Signed-off-by: Omar Sandoval <[email protected]>
---
include/linux/uio.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 7188029..87a47b3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -10,6 +10,7 @@
#define __LINUX_UIO_H
#include <linux/kernel.h>
+#include <linux/fs.h>
#include <uapi/linux/uio.h>
struct page;
@@ -111,6 +112,15 @@ static inline bool iter_is_iovec(struct iov_iter *i)
}
/*
+ * Get one of READ or WRITE out of iter->type without any other flags OR'd in
+ * with it.
+ */
+static inline int iov_iter_rw(const struct iov_iter *i)
+{
+ return i->type & RW_MASK;
+}
+
+/*
* Cap the iov_iter by given limit; note that the second argument is
* *not* the new size - it's upper limit for such. Passing it a value
* greater than the amount of data in iov_iter is fine - it'll just do
--
2.3.3
Now that no one is using rw, remove it completely.
Signed-off-by: Omar Sandoval <[email protected]>
---
Documentation/filesystems/Locking | 2 +-
Documentation/filesystems/vfs.txt | 2 +-
drivers/staging/lustre/lustre/llite/rw26.c | 4 ++--
fs/9p/vfs_addr.c | 2 +-
fs/affs/file.c | 3 +--
fs/block_dev.c | 3 +--
fs/btrfs/inode.c | 4 ++--
fs/ceph/addr.c | 3 +--
fs/cifs/file.c | 3 +--
fs/exofs/inode.c | 4 ++--
fs/ext2/inode.c | 3 +--
fs/ext3/inode.c | 4 ++--
fs/ext4/inode.c | 4 ++--
fs/f2fs/data.c | 4 ++--
fs/fat/inode.c | 3 +--
fs/fuse/file.c | 3 +--
fs/gfs2/aops.c | 4 ++--
fs/hfs/inode.c | 4 ++--
fs/hfsplus/inode.c | 4 ++--
fs/jfs/inode.c | 4 ++--
fs/nfs/direct.c | 2 +-
fs/nilfs2/inode.c | 3 +--
fs/ocfs2/aops.c | 4 +---
fs/reiserfs/inode.c | 4 ++--
fs/udf/file.c | 3 +--
fs/udf/inode.c | 3 +--
fs/xfs/xfs_aops.c | 1 -
include/linux/fs.h | 2 +-
include/linux/nfs_fs.h | 2 +-
mm/filemap.c | 4 ++--
mm/page_io.c | 4 +---
31 files changed, 42 insertions(+), 57 deletions(-)
diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
index f91926f..b38abaf 100644
--- a/Documentation/filesystems/Locking
+++ b/Documentation/filesystems/Locking
@@ -196,7 +196,7 @@ prototypes:
void (*invalidatepage) (struct page *, unsigned int, unsigned int);
int (*releasepage) (struct page *, int);
void (*freepage)(struct page *);
- int (*direct_IO)(int, struct kiocb *, struct iov_iter *iter, loff_t offset);
+ int (*direct_IO)(struct kiocb *, struct iov_iter *iter, loff_t offset);
int (*migratepage)(struct address_space *, struct page *, struct page *);
int (*launder_page)(struct page *);
int (*is_partially_uptodate)(struct page *, unsigned long, unsigned long);
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 966b228..d8ebc3c 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -590,7 +590,7 @@ struct address_space_operations {
void (*invalidatepage) (struct page *, unsigned int, unsigned int);
int (*releasepage) (struct page *, int);
void (*freepage)(struct page *);
- ssize_t (*direct_IO)(int, struct kiocb *, struct iov_iter *iter, loff_t offset);
+ ssize_t (*direct_IO)(struct kiocb *, struct iov_iter *iter, loff_t offset);
/* migrate the contents of a page to the specified target */
int (*migratepage) (struct page *, struct page *);
int (*launder_page) (struct page *);
diff --git a/drivers/staging/lustre/lustre/llite/rw26.c b/drivers/staging/lustre/lustre/llite/rw26.c
index 3aa9de6..0d7ce6b 100644
--- a/drivers/staging/lustre/lustre/llite/rw26.c
+++ b/drivers/staging/lustre/lustre/llite/rw26.c
@@ -359,8 +359,8 @@ static ssize_t ll_direct_IO_26_seg(const struct lu_env *env, struct cl_io *io,
* up to 22MB for 128kB kmalloc and up to 682MB for 4MB kmalloc. */
#define MAX_DIO_SIZE ((MAX_MALLOC / sizeof(struct brw_page) * PAGE_CACHE_SIZE) & \
~(DT_MAX_BRW_SIZE - 1))
-static ssize_t ll_direct_IO_26(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t file_offset)
+static ssize_t ll_direct_IO_26(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t file_offset)
{
struct lu_env *env;
struct cl_io *io;
diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
index eb14e05..b298a90 100644
--- a/fs/9p/vfs_addr.c
+++ b/fs/9p/vfs_addr.c
@@ -259,7 +259,7 @@ static int v9fs_launder_page(struct page *page)
*
*/
static ssize_t
-v9fs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter, loff_t pos)
+v9fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t pos)
{
/*
* FIXME
diff --git a/fs/affs/file.c b/fs/affs/file.c
index 0314acd..ef5dcb4 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -389,8 +389,7 @@ static void affs_write_failed(struct address_space *mapping, loff_t to)
}
static ssize_t
-affs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t offset)
+affs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index e3a3125..02ba7b0 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -147,8 +147,7 @@ blkdev_get_block(struct inode *inode, sector_t iblock,
}
static ssize_t
-blkdev_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t offset)
+blkdev_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5c17f61..ffad756 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8070,8 +8070,8 @@ out:
return retval;
}
-static ssize_t btrfs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t btrfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index fd5599d..155ab9c 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1198,8 +1198,7 @@ static int ceph_write_end(struct file *file, struct address_space *mapping,
* intercept O_DIRECT reads and writes early, this function should
* never get called.
*/
-static ssize_t ceph_direct_io(int rw, struct kiocb *iocb,
- struct iov_iter *iter,
+static ssize_t ceph_direct_io(struct kiocb *iocb, struct iov_iter *iter,
loff_t pos)
{
WARN_ON(1);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a94b3e6..83ae6cc 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3876,8 +3876,7 @@ void cifs_oplock_break(struct work_struct *work)
* Direct IO is not yet supported in the cached mode.
*/
static ssize_t
-cifs_direct_io(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t pos)
+cifs_direct_io(struct kiocb *iocb, struct iov_iter *iter, loff_t pos)
{
/*
* FIXME
diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index a198e94..35073aa 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -963,8 +963,8 @@ static void exofs_invalidatepage(struct page *page, unsigned int offset,
/* TODO: Should be easy enough to do proprly */
-static ssize_t exofs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t exofs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
return 0;
}
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index d59d21f..ac2a281 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -851,8 +851,7 @@ static sector_t ext2_bmap(struct address_space *mapping, sector_t block)
}
static ssize_t
-ext2_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t offset)
+ext2_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 8832f2d..91ac933 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -1820,8 +1820,8 @@ static int ext3_releasepage(struct page *page, gfp_t wait)
* crashes then stale disk data _may_ be exposed inside the file. But current
* VFS code falls back into buffered path in that case so we are safe.
*/
-static ssize_t ext3_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t ext3_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 1e63c19..8a5d3a1 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3090,8 +3090,8 @@ retake_lock:
return ret;
}
-static ssize_t ext4_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t ext4_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9c8f0c0..8e58b43 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1135,8 +1135,8 @@ static int check_direct_IO(struct inode *inode, struct iov_iter *iter,
return 0;
}
-static ssize_t f2fs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index cfc3461..eeea430 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -246,8 +246,7 @@ static int fat_write_end(struct file *file, struct address_space *mapping,
return err;
}
-static ssize_t fat_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter,
+static ssize_t fat_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
loff_t offset)
{
struct file *file = iocb->ki_filp;
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d9679d4..1d11ce2 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2798,8 +2798,7 @@ static inline loff_t fuse_round_up(loff_t off)
}
static ssize_t
-fuse_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t offset)
+fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
{
ssize_t ret = 0;
struct file *file = iocb->ki_filp;
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 4544200..5cee2ab 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -1038,8 +1038,8 @@ static int gfs2_ok_for_dio(struct gfs2_inode *ip, loff_t offset)
-static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t gfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 754d31f..1f2e376 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -124,8 +124,8 @@ static int hfs_releasepage(struct page *page, gfp_t mask)
return res ? try_to_free_buffers(page) : 0;
}
-static ssize_t hfs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t hfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index fb0e914..ee9fb03 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -122,8 +122,8 @@ static int hfsplus_releasepage(struct page *page, gfp_t mask)
return res ? try_to_free_buffers(page) : 0;
}
-static ssize_t hfsplus_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t hfsplus_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index 62a2e39..33b10c5 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -330,8 +330,8 @@ static sector_t jfs_bmap(struct address_space *mapping, sector_t block)
return generic_block_bmap(mapping, block, jfs_get_block);
}
-static ssize_t jfs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t jfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index 541dcc0..6db73a0 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -251,7 +251,7 @@ static int nfs_direct_cmp_commit_data_verf(struct nfs_direct_req *dreq,
* shunt off direct read and write requests before the VFS gets them,
* so this method is only ever called for swap.
*/
-ssize_t nfs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter, loff_t pos)
+ssize_t nfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t pos)
{
struct inode *inode = iocb->ki_filp->f_mapping->host;
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 10c2e39..946de2f 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -305,8 +305,7 @@ static int nilfs_write_end(struct file *file, struct address_space *mapping,
}
static ssize_t
-nilfs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
- loff_t offset)
+nilfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index a4a1b9a..a804143 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -817,9 +817,7 @@ out:
return ret;
}
-static ssize_t ocfs2_direct_IO(int rw,
- struct kiocb *iocb,
- struct iov_iter *iter,
+static ssize_t ocfs2_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
loff_t offset)
{
struct file *file = iocb->ki_filp;
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index 6d253da..4f1a8f3 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -3278,8 +3278,8 @@ static int reiserfs_releasepage(struct page *page, gfp_t unused_gfp_flags)
* We thank Mingming Cao for helping us understand in great detail what
* to do in this section of the code.
*/
-static ssize_t reiserfs_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter, loff_t offset)
+static ssize_t reiserfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
+ loff_t offset)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 08f3555..d7ad099 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -100,8 +100,7 @@ static int udf_adinicb_write_begin(struct file *file,
return 0;
}
-static ssize_t udf_adinicb_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter,
+static ssize_t udf_adinicb_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
loff_t offset)
{
/* Fallback to buffered I/O. */
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 91e7e1c..20d7bc9 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -215,8 +215,7 @@ static int udf_write_begin(struct file *file, struct address_space *mapping,
return ret;
}
-static ssize_t udf_direct_IO(int rw, struct kiocb *iocb,
- struct iov_iter *iter,
+static ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
loff_t offset)
{
struct file *file = iocb->ki_filp;
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index fd6e6f5..e362299 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1496,7 +1496,6 @@ xfs_end_io_direct_write(
STATIC ssize_t
xfs_vm_direct_IO(
- int rw,
struct kiocb *iocb,
struct iov_iter *iter,
loff_t offset)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8fcda74..19b8adf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -361,7 +361,7 @@ struct address_space_operations {
void (*invalidatepage) (struct page *, unsigned int, unsigned int);
int (*releasepage) (struct page *, gfp_t);
void (*freepage)(struct page *);
- ssize_t (*direct_IO)(int, struct kiocb *, struct iov_iter *iter, loff_t offset);
+ ssize_t (*direct_IO)(struct kiocb *, struct iov_iter *iter, loff_t offset);
/*
* migrate the contents of a page to the specified target. If
* migrate_mode is MIGRATE_ASYNC, it must not block.
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index b01ccf3..3d1b0d2 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -447,7 +447,7 @@ static inline struct rpc_cred *nfs_file_cred(struct file *file)
/*
* linux/fs/nfs/direct.c
*/
-extern ssize_t nfs_direct_IO(int, struct kiocb *, struct iov_iter *, loff_t);
+extern ssize_t nfs_direct_IO(struct kiocb *, struct iov_iter *, loff_t);
extern ssize_t nfs_file_direct_read(struct kiocb *iocb,
struct iov_iter *iter,
loff_t pos);
diff --git a/mm/filemap.c b/mm/filemap.c
index ad72420..cb63a50 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1708,7 +1708,7 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
pos + count - 1);
if (!retval) {
struct iov_iter data = *iter;
- retval = mapping->a_ops->direct_IO(READ, iocb, &data, pos);
+ retval = mapping->a_ops->direct_IO(iocb, &data, pos);
}
if (retval > 0) {
@@ -2396,7 +2396,7 @@ generic_file_direct_write(struct kiocb *iocb, struct iov_iter *from, loff_t pos)
}
data = *from;
- written = mapping->a_ops->direct_IO(WRITE, iocb, &data, pos);
+ written = mapping->a_ops->direct_IO(iocb, &data, pos);
/*
* Finally, try again to invalidate clean pages which might have been
diff --git a/mm/page_io.c b/mm/page_io.c
index e604580..14ca9d3 100644
--- a/mm/page_io.c
+++ b/mm/page_io.c
@@ -278,9 +278,7 @@ int __swap_writepage(struct page *page, struct writeback_control *wbc,
set_page_writeback(page);
unlock_page(page);
- ret = mapping->a_ops->direct_IO(ITER_BVEC | WRITE,
- &kiocb, &from,
- kiocb.ki_pos);
+ ret = mapping->a_ops->direct_IO(&kiocb, &from, kiocb.ki_pos);
if (ret == PAGE_SIZE) {
count_vm_event(PSWPOUT);
ret = 0;
--
2.3.3
And use iov_iter_rw() instead.
Signed-off-by: Omar Sandoval <[email protected]>
---
fs/dax.c | 27 +++++++++++++--------------
fs/ext2/inode.c | 4 ++--
fs/ext4/indirect.c | 4 ++--
fs/ext4/inode.c | 2 +-
include/linux/fs.h | 4 ++--
5 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index ed1619e..a278469 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -98,9 +98,9 @@ static bool buffer_size_valid(struct buffer_head *bh)
return bh->b_state != 0;
}
-static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
- loff_t start, loff_t end, get_block_t get_block,
- struct buffer_head *bh)
+static ssize_t dax_io(struct inode *inode, struct iov_iter *iter,
+ loff_t start, loff_t end, get_block_t get_block,
+ struct buffer_head *bh)
{
ssize_t retval = 0;
loff_t pos = start;
@@ -109,7 +109,7 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
void *addr;
bool hole = false;
- if (rw != WRITE)
+ if (iov_iter_rw(iter) != WRITE)
end = min(end, i_size_read(inode));
while (pos < end) {
@@ -124,7 +124,7 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
bh->b_size = PAGE_ALIGN(end - pos);
bh->b_state = 0;
retval = get_block(inode, block, bh,
- rw == WRITE);
+ iov_iter_rw(iter) == WRITE);
if (retval)
break;
if (!buffer_size_valid(bh))
@@ -137,7 +137,7 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
bh->b_size -= done;
}
- hole = (rw != WRITE) && !buffer_written(bh);
+ hole = iov_iter_rw(iter) != WRITE && !buffer_written(bh);
if (hole) {
addr = NULL;
size = bh->b_size - first;
@@ -154,7 +154,7 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
max = min(pos + size, end);
}
- if (rw == WRITE)
+ if (iov_iter_rw(iter) == WRITE)
len = copy_from_iter(addr, max - pos, iter);
else if (!hole)
len = copy_to_iter(addr, max - pos, iter);
@@ -173,7 +173,6 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
/**
* dax_do_io - Perform I/O to a DAX file
- * @rw: READ to read or WRITE to write
* @iocb: The control block for this I/O
* @inode: The file which the I/O is directed at
* @iter: The addresses to do I/O from or to
@@ -189,9 +188,9 @@ static ssize_t dax_io(int rw, struct inode *inode, struct iov_iter *iter,
* As with do_blockdev_direct_IO(), we increment i_dio_count while the I/O
* is in progress.
*/
-ssize_t dax_do_io(int rw, struct kiocb *iocb, struct inode *inode,
- struct iov_iter *iter, loff_t pos,
- get_block_t get_block, dio_iodone_t end_io, int flags)
+ssize_t dax_do_io(struct kiocb *iocb, struct inode *inode,
+ struct iov_iter *iter, loff_t pos, get_block_t get_block,
+ dio_iodone_t end_io, int flags)
{
struct buffer_head bh;
ssize_t retval = -EINVAL;
@@ -199,7 +198,7 @@ ssize_t dax_do_io(int rw, struct kiocb *iocb, struct inode *inode,
memset(&bh, 0, sizeof(bh));
- if ((flags & DIO_LOCKING) && (rw == READ)) {
+ if ((flags & DIO_LOCKING) && iov_iter_rw(iter) == READ) {
struct address_space *mapping = inode->i_mapping;
mutex_lock(&inode->i_mutex);
retval = filemap_write_and_wait_range(mapping, pos, end - 1);
@@ -212,9 +211,9 @@ ssize_t dax_do_io(int rw, struct kiocb *iocb, struct inode *inode,
/* Protects against truncate */
atomic_inc(&inode->i_dio_count);
- retval = dax_io(rw, inode, iter, pos, end, get_block, &bh);
+ retval = dax_io(inode, iter, pos, end, get_block, &bh);
- if ((flags & DIO_LOCKING) && (rw == READ))
+ if ((flags & DIO_LOCKING) && iov_iter_rw(iter) == READ)
mutex_unlock(&inode->i_mutex);
if ((retval > 0) && end_io)
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 00979a6..cf95bda 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -861,8 +861,8 @@ ext2_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
ssize_t ret;
if (IS_DAX(inode))
- ret = dax_do_io(rw, iocb, inode, iter, offset, ext2_get_block,
- NULL, DIO_LOCKING);
+ ret = dax_do_io(iocb, inode, iter, offset, ext2_get_block, NULL,
+ DIO_LOCKING);
else
ret = blockdev_direct_IO(iocb, inode, iter, offset,
ext2_get_block);
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 57135a7..86e8765 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -690,7 +690,7 @@ retry:
goto locked;
}
if (IS_DAX(inode))
- ret = dax_do_io(rw, iocb, inode, iter, offset,
+ ret = dax_do_io(iocb, inode, iter, offset,
ext4_get_block, NULL, 0);
else
ret = __blockdev_direct_IO(iocb, inode,
@@ -701,7 +701,7 @@ retry:
} else {
locked:
if (IS_DAX(inode))
- ret = dax_do_io(rw, iocb, inode, iter, offset,
+ ret = dax_do_io(iocb, inode, iter, offset,
ext4_get_block, NULL, DIO_LOCKING);
else
ret = blockdev_direct_IO(iocb, inode, iter, offset,
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 81f8f10..094cc4a 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3035,7 +3035,7 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
dio_flags = DIO_LOCKING;
}
if (IS_DAX(inode))
- ret = dax_do_io(rw, iocb, inode, iter, offset, get_block_func,
+ ret = dax_do_io(iocb, inode, iter, offset, get_block_func,
ext4_end_io_dio, dio_flags);
else
ret = __blockdev_direct_IO(iocb, inode,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index ba1b580..8fcda74 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2591,8 +2591,8 @@ extern loff_t fixed_size_llseek(struct file *file, loff_t offset,
extern int generic_file_open(struct inode * inode, struct file * filp);
extern int nonseekable_open(struct inode * inode, struct file * filp);
-ssize_t dax_do_io(int rw, struct kiocb *, struct inode *, struct iov_iter *,
- loff_t, get_block_t, dio_iodone_t, int flags);
+ssize_t dax_do_io(struct kiocb *, struct inode *, struct iov_iter *, loff_t,
+ get_block_t, dio_iodone_t, int flags);
int dax_clear_blocks(struct inode *, sector_t block, long size);
int dax_zero_page_range(struct inode *, loff_t from, unsigned len, get_block_t);
int dax_truncate_page(struct inode *, loff_t from, get_block_t);
--
2.3.3
Most filesystems call through to these at some point, so we'll start
here.
Signed-off-by: Omar Sandoval <[email protected]>
---
fs/affs/file.c | 2 +-
fs/block_dev.c | 5 ++---
fs/btrfs/inode.c | 8 ++++----
fs/direct-io.c | 39 ++++++++++++++++++---------------------
fs/ext2/inode.c | 2 +-
fs/ext3/inode.c | 2 +-
fs/ext4/indirect.c | 11 ++++++-----
fs/ext4/inode.c | 2 +-
fs/f2fs/data.c | 2 +-
fs/fat/inode.c | 2 +-
fs/gfs2/aops.c | 5 ++---
fs/hfs/inode.c | 2 +-
fs/hfsplus/inode.c | 3 +--
fs/jfs/inode.c | 2 +-
fs/nilfs2/inode.c | 3 +--
fs/ocfs2/aops.c | 16 +++++++---------
fs/reiserfs/inode.c | 2 +-
fs/udf/inode.c | 2 +-
fs/xfs/xfs_aops.c | 9 ++++-----
include/linux/fs.h | 22 ++++++++++++----------
20 files changed, 67 insertions(+), 74 deletions(-)
diff --git a/fs/affs/file.c b/fs/affs/file.c
index d2468bf..f3028aa 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -405,7 +405,7 @@ affs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
return 0;
}
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, affs_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, affs_get_block);
if (ret < 0 && (rw & WRITE))
affs_write_failed(mapping, offset + count);
return ret;
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 975266b..e3a3125 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -153,9 +153,8 @@ blkdev_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
- return __blockdev_direct_IO(rw, iocb, inode, I_BDEV(inode), iter,
- offset, blkdev_get_block,
- NULL, NULL, 0);
+ return __blockdev_direct_IO(iocb, inode, I_BDEV(inode), iter, offset,
+ blkdev_get_block, NULL, NULL, 0);
}
int __sync_blockdev(struct block_device *bdev, int wait)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index da828cf..d6413f4 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -8119,10 +8119,10 @@ static ssize_t btrfs_direct_IO(int rw, struct kiocb *iocb,
wakeup = false;
}
- ret = __blockdev_direct_IO(rw, iocb, inode,
- BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
- iter, offset, btrfs_get_blocks_direct, NULL,
- btrfs_submit_direct, flags);
+ ret = __blockdev_direct_IO(iocb, inode,
+ BTRFS_I(inode)->root->fs_info->fs_devices->latest_bdev,
+ iter, offset, btrfs_get_blocks_direct, NULL,
+ btrfs_submit_direct, flags);
if (rw & WRITE) {
if (ret < 0 && ret != -EIOCBQUEUED)
btrfs_delalloc_release_space(inode, count);
diff --git a/fs/direct-io.c b/fs/direct-io.c
index e181b6b..02a87d9 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1094,10 +1094,10 @@ static inline int drop_refcount(struct dio *dio)
* for the whole file.
*/
static inline ssize_t
-do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
- struct block_device *bdev, struct iov_iter *iter, loff_t offset,
- get_block_t get_block, dio_iodone_t end_io,
- dio_submit_t submit_io, int flags)
+do_blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
+ struct block_device *bdev, struct iov_iter *iter,
+ loff_t offset, get_block_t get_block, dio_iodone_t end_io,
+ dio_submit_t submit_io, int flags)
{
unsigned i_blkbits = ACCESS_ONCE(inode->i_blkbits);
unsigned blkbits = i_blkbits;
@@ -1111,9 +1111,6 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
struct blk_plug plug;
unsigned long align = offset | iov_iter_alignment(iter);
- if (rw & WRITE)
- rw = WRITE_ODIRECT;
-
/*
* Avoid references to bdev if not absolutely needed to give
* the early prefetch in the caller enough time.
@@ -1128,7 +1125,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
}
/* watch out for a 0 len io from a tricksy fs */
- if (rw == READ && !iov_iter_count(iter))
+ if (iov_iter_rw(iter) == READ && !iov_iter_count(iter))
return 0;
dio = kmem_cache_alloc(dio_cache, GFP_KERNEL);
@@ -1144,7 +1141,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
dio->flags = flags;
if (dio->flags & DIO_LOCKING) {
- if (rw == READ) {
+ if (iov_iter_rw(iter) == READ) {
struct address_space *mapping =
iocb->ki_filp->f_mapping;
@@ -1170,19 +1167,19 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
if (is_sync_kiocb(iocb))
dio->is_async = false;
else if (!(dio->flags & DIO_ASYNC_EXTEND) &&
- (rw & WRITE) && end > i_size_read(inode))
+ iov_iter_rw(iter) == WRITE && end > i_size_read(inode))
dio->is_async = false;
else
dio->is_async = true;
dio->inode = inode;
- dio->rw = rw;
+ dio->rw = iov_iter_rw(iter) == WRITE ? WRITE_ODIRECT : READ;
/*
* For AIO O_(D)SYNC writes we need to defer completions to a workqueue
* so that we can call ->fsync.
*/
- if (dio->is_async && (rw & WRITE) &&
+ if (dio->is_async && iov_iter_rw(iter) == WRITE &&
((iocb->ki_filp->f_flags & O_DSYNC) ||
IS_SYNC(iocb->ki_filp->f_mapping->host))) {
retval = dio_set_defer_completion(dio);
@@ -1275,7 +1272,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
* we can let i_mutex go now that its achieved its purpose
* of protecting us from looking up uninitialized blocks.
*/
- if (rw == READ && (dio->flags & DIO_LOCKING))
+ if (iov_iter_rw(iter) == READ && (dio->flags & DIO_LOCKING))
mutex_unlock(&dio->inode->i_mutex);
/*
@@ -1287,7 +1284,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
*/
BUG_ON(retval == -EIOCBQUEUED);
if (dio->is_async && retval == 0 && dio->result &&
- (rw == READ || dio->result == count))
+ (iov_iter_rw(iter) == READ || dio->result == count))
retval = -EIOCBQUEUED;
else
dio_await_completion(dio);
@@ -1301,11 +1298,11 @@ out:
return retval;
}
-ssize_t
-__blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
- struct block_device *bdev, struct iov_iter *iter, loff_t offset,
- get_block_t get_block, dio_iodone_t end_io,
- dio_submit_t submit_io, int flags)
+ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
+ struct block_device *bdev, struct iov_iter *iter,
+ loff_t offset, get_block_t get_block,
+ dio_iodone_t end_io, dio_submit_t submit_io,
+ int flags)
{
/*
* The block device state is needed in the end to finally
@@ -1319,8 +1316,8 @@ __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
prefetch(bdev->bd_queue);
prefetch((char *)bdev->bd_queue + SMP_CACHE_BYTES);
- return do_blockdev_direct_IO(rw, iocb, inode, bdev, iter, offset,
- get_block, end_io, submit_io, flags);
+ return do_blockdev_direct_IO(iocb, inode, bdev, iter, offset, get_block,
+ end_io, submit_io, flags);
}
EXPORT_SYMBOL(__blockdev_direct_IO);
diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 6434bc0..00979a6 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -864,7 +864,7 @@ ext2_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
ret = dax_do_io(rw, iocb, inode, iter, offset, ext2_get_block,
NULL, DIO_LOCKING);
else
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset,
+ ret = blockdev_direct_IO(iocb, inode, iter, offset,
ext2_get_block);
if (ret < 0 && (rw & WRITE))
ext2_write_failed(mapping, offset + count);
diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index 2c6ccc4..28bba7d 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -1856,7 +1856,7 @@ static ssize_t ext3_direct_IO(int rw, struct kiocb *iocb,
}
retry:
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, ext3_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, ext3_get_block);
/*
* In case of error extending write may have instantiated a few
* blocks outside i_size. Trim these off again.
diff --git a/fs/ext4/indirect.c b/fs/ext4/indirect.c
index 45fe924..57135a7 100644
--- a/fs/ext4/indirect.c
+++ b/fs/ext4/indirect.c
@@ -693,9 +693,10 @@ retry:
ret = dax_do_io(rw, iocb, inode, iter, offset,
ext4_get_block, NULL, 0);
else
- ret = __blockdev_direct_IO(rw, iocb, inode,
- inode->i_sb->s_bdev, iter, offset,
- ext4_get_block, NULL, NULL, 0);
+ ret = __blockdev_direct_IO(iocb, inode,
+ inode->i_sb->s_bdev, iter,
+ offset, ext4_get_block, NULL,
+ NULL, 0);
inode_dio_done(inode);
} else {
locked:
@@ -703,8 +704,8 @@ locked:
ret = dax_do_io(rw, iocb, inode, iter, offset,
ext4_get_block, NULL, DIO_LOCKING);
else
- ret = blockdev_direct_IO(rw, iocb, inode, iter,
- offset, ext4_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset,
+ ext4_get_block);
if (unlikely((rw & WRITE) && ret < 0)) {
loff_t isize = i_size_read(inode);
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5cb9a21..81f8f10 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3038,7 +3038,7 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
ret = dax_do_io(rw, iocb, inode, iter, offset, get_block_func,
ext4_end_io_dio, dio_flags);
else
- ret = __blockdev_direct_IO(rw, iocb, inode,
+ ret = __blockdev_direct_IO(iocb, inode,
inode->i_sb->s_bdev, iter, offset,
get_block_func,
ext4_end_io_dio, NULL, dio_flags);
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 985ed02..d2addce 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1159,7 +1159,7 @@ static ssize_t f2fs_direct_IO(int rw, struct kiocb *iocb,
if (rw & WRITE)
__allocate_data_blocks(inode, offset, count);
- err = blockdev_direct_IO(rw, iocb, inode, iter, offset, get_data_block);
+ err = blockdev_direct_IO(iocb, inode, iter, offset, get_data_block);
if (err < 0 && (rw & WRITE))
f2fs_write_failed(mapping, offset + count);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 497c7c5..a969a34 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -275,7 +275,7 @@ static ssize_t fat_direct_IO(int rw, struct kiocb *iocb,
* FAT need to use the DIO_LOCKING for avoiding the race
* condition of fat_get_block() and ->truncate().
*/
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, fat_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, fat_get_block);
if (ret < 0 && (rw & WRITE))
fat_write_failed(mapping, offset + count);
diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 4ad4f94..fc691f2 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -1095,9 +1095,8 @@ static ssize_t gfs2_direct_IO(int rw, struct kiocb *iocb,
truncate_inode_pages_range(mapping, lstart, end);
}
- rv = __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev,
- iter, offset,
- gfs2_get_block_direct, NULL, NULL, 0);
+ rv = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
+ offset, gfs2_get_block_direct, NULL, NULL, 0);
out:
gfs2_glock_dq(&gh);
gfs2_holder_uninit(&gh);
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index d0929bc..d367253 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -133,7 +133,7 @@ static ssize_t hfs_direct_IO(int rw, struct kiocb *iocb,
size_t count = iov_iter_count(iter);
ssize_t ret;
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, hfs_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, hfs_get_block);
/*
* In case of error extending write may have instantiated a few
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 0cf786f..a7ec37d 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -131,8 +131,7 @@ static ssize_t hfsplus_direct_IO(int rw, struct kiocb *iocb,
size_t count = iov_iter_count(iter);
ssize_t ret;
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset,
- hfsplus_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, hfsplus_get_block);
/*
* In case of error extending write may have instantiated a few
diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
index bd3df1c..88271ac 100644
--- a/fs/jfs/inode.c
+++ b/fs/jfs/inode.c
@@ -339,7 +339,7 @@ static ssize_t jfs_direct_IO(int rw, struct kiocb *iocb,
size_t count = iov_iter_count(iter);
ssize_t ret;
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, jfs_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, jfs_get_block);
/*
* In case of error extending write may have instantiated a few
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 8b59695..76587c3 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -318,8 +318,7 @@ nilfs_direct_IO(int rw, struct kiocb *iocb, struct iov_iter *iter,
return 0;
/* Needs synchronization with the cleaner */
- size = blockdev_direct_IO(rw, iocb, inode, iter, offset,
- nilfs_get_block);
+ size = blockdev_direct_IO(iocb, inode, iter, offset, nilfs_get_block);
/*
* In case of error extending write may have instantiated a few
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index 44db180..162a2a8 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -737,10 +737,9 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb,
di_bh = NULL;
}
- written = __blockdev_direct_IO(WRITE, iocb, inode, inode->i_sb->s_bdev,
- iter, offset,
- ocfs2_direct_IO_get_blocks,
- ocfs2_dio_end_io, NULL, 0);
+ written = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
+ offset, ocfs2_direct_IO_get_blocks,
+ ocfs2_dio_end_io, NULL, 0);
if (unlikely(written < 0)) {
loff_t i_size = i_size_read(inode);
@@ -843,11 +842,10 @@ static ssize_t ocfs2_direct_IO(int rw,
return 0;
if (rw == READ)
- return __blockdev_direct_IO(rw, iocb, inode,
- inode->i_sb->s_bdev,
- iter, offset,
- ocfs2_direct_IO_get_blocks,
- ocfs2_dio_end_io, NULL, 0);
+ return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev,
+ iter, offset,
+ ocfs2_direct_IO_get_blocks,
+ ocfs2_dio_end_io, NULL, 0);
else
return ocfs2_direct_IO_write(iocb, iter, offset);
}
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index e72401e..254e800 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -3286,7 +3286,7 @@ static ssize_t reiserfs_direct_IO(int rw, struct kiocb *iocb,
size_t count = iov_iter_count(iter);
ssize_t ret;
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset,
+ ret = blockdev_direct_IO(iocb, inode, iter, offset,
reiserfs_get_blocks_direct_io);
/*
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index a445d59..bcb0082 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -225,7 +225,7 @@ static ssize_t udf_direct_IO(int rw, struct kiocb *iocb,
size_t count = iov_iter_count(iter);
ssize_t ret;
- ret = blockdev_direct_IO(rw, iocb, inode, iter, offset, udf_get_block);
+ ret = blockdev_direct_IO(iocb, inode, iter, offset, udf_get_block);
if (unlikely(ret < 0 && (rw & WRITE)))
udf_write_failed(mapping, offset + count);
return ret;
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 3a9b7a1..fdb1353 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -1505,14 +1505,13 @@ xfs_vm_direct_IO(
struct block_device *bdev = xfs_find_bdev_for_inode(inode);
if (rw & WRITE) {
- return __blockdev_direct_IO(rw, iocb, inode, bdev, iter,
- offset, xfs_get_blocks_direct,
+ return __blockdev_direct_IO(iocb, inode, bdev, iter, offset,
+ xfs_get_blocks_direct,
xfs_end_io_direct_write, NULL,
DIO_ASYNC_EXTEND);
}
- return __blockdev_direct_IO(rw, iocb, inode, bdev, iter,
- offset, xfs_get_blocks_direct,
- NULL, NULL, 0);
+ return __blockdev_direct_IO(iocb, inode, bdev, iter, offset,
+ xfs_get_blocks_direct, NULL, NULL, 0);
}
/*
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b4d71b5..ba1b580 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2616,16 +2616,18 @@ enum {
void dio_end_io(struct bio *bio, int error);
-ssize_t __blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
- struct block_device *bdev, struct iov_iter *iter, loff_t offset,
- get_block_t get_block, dio_iodone_t end_io,
- dio_submit_t submit_io, int flags);
-
-static inline ssize_t blockdev_direct_IO(int rw, struct kiocb *iocb,
- struct inode *inode, struct iov_iter *iter, loff_t offset,
- get_block_t get_block)
-{
- return __blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iter,
+ssize_t __blockdev_direct_IO(struct kiocb *iocb, struct inode *inode,
+ struct block_device *bdev, struct iov_iter *iter,
+ loff_t offset, get_block_t get_block,
+ dio_iodone_t end_io, dio_submit_t submit_io,
+ int flags);
+
+static inline ssize_t blockdev_direct_IO(struct kiocb *iocb,
+ struct inode *inode,
+ struct iov_iter *iter, loff_t offset,
+ get_block_t get_block)
+{
+ return __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter,
offset, get_block, NULL, NULL,
DIO_LOCKING | DIO_SKIP_HOLES);
}
--
2.3.3
On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> Get either READ or WRITE out of iter->type.
Umm...
> + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> + * with it.
> + */
> +static inline int iov_iter_rw(const struct iov_iter *i)
> +{
> + return i->type & RW_MASK;
> +}
TBH, I would turn that into a macro. Reason: indirect includes.
How about
#define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
Should do you all the type safety of inline function and avoids the need
to include fs.h in uio.h; _users_ of iov_iter_rw() obviously still need
fs.h, but such places always used to...
On Mon, Mar 16, 2015 at 04:33:48AM -0700, Omar Sandoval wrote:
> Hi,
>
> Al, here's some cleanup that you mentioned back in December that I got
> around to (https://lkml.org/lkml/2014/12/15/28).
>
> In summary, the rw parameter to a_ops->direct_IO() is redundant with
> .type in struct iov_iter. Additionally, rw is inconsistently checked for
> being a WRITE; some filesystems do rw == WRITE, others do rw & WRITE,
> and others do both within the same function :) The distinction is that
> swapout may OR in the ITER_BVEC flag in the rw passed to ->direct_IO(),
> so the two are not equivalent (although this really only happens for
> swap-over-NFS, but it's scary nonetheless). After looking through all of
> these, it definitely looks like every check means for ANY write, not
> just non-kernel writes.
>
> So, the solution presented here is:
>
> - Add a helper, iov_iter_rw(), which always returns either READ or
> WRITE, no ITER_.* or REQ_.* nonsense mixed in. For consistency, the
> return value is always checked for equality
TBH, I'm not sure I like such calling conventions, but I guess we can
live with that.
> I decided to squish all of the filesystems together in patch 4 to avoid
> inundating the mailing lists with 20+ mostly two-line patches, but I can
> split those out if that's any better. Additionally, patch 1 pulls fs.h
> into uio.h, which seems undesirable.
... and easily avoided if you use a macro instead of inline, without losing
type safety or getting double evaluation, etc.
Look: 0 ? (struct T *)0 : (x) always evaluates to x. Now look at 6.5.15p3 in
C99: the second and the third arguments are both pointers, so we are left with
p3.4 (both arguments are pointers to qualified or unqualified versions of
compatible types), p3.5 (one operand is a pointer and another null pointer
constant) and p3.6 (one operand is a pointer to an object or incomplete type,
and the other is a pointer to qualified or unqualied version of void.
The first variant means that x is a pointer to qualified or unqualified
struct T; the type of result is, per 6.5.15p6, the same as that of x.
The second variant means that x is a null pointer constant ((struct T *)0 isn't
one) and result is a null pointer to T.
The third one means that x is a pointer to qualified or unqualified void.
The type of result is the same as that of x.
Now note that your variant is no better wrt type safety; worse, actually, since
it does accept any pointer to void. (0 ? (struct iov_iter *)0 : (x))->type
will reject those. And we obviously don't have double evaluation here either.
On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > Get either READ or WRITE out of iter->type.
>
> Umm...
>
> > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > + * with it.
> > + */
> > +static inline int iov_iter_rw(const struct iov_iter *i)
> > +{
> > + return i->type & RW_MASK;
> > +}
>
> TBH, I would turn that into a macro. Reason: indirect includes.
Agreed, but the proposed define is rather cryptic and I was not able to
understand the meaning on the first glance.
> #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
This worked for me, does not compile with anything else than
'struct iov_iter*' as i:
#define iov_iter_rw(i) ({ \
struct iov_iter __iter = *(i); \
(i)->type & RW_MASK; \
})
The assignment is optimized out.
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> On Mon, Mar 16, 2015 at 05:36:05PM +0000, Al Viro wrote:
> > On Mon, Mar 16, 2015 at 04:33:49AM -0700, Omar Sandoval wrote:
> > > Get either READ or WRITE out of iter->type.
> >
> > Umm...
> >
> > > + * Get one of READ or WRITE out of iter->type without any other flags OR'd in
> > > + * with it.
> > > + */
> > > +static inline int iov_iter_rw(const struct iov_iter *i)
> > > +{
> > > + return i->type & RW_MASK;
> > > +}
> >
> > TBH, I would turn that into a macro. Reason: indirect includes.
>
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
[-cc individual fs maintainers to avoid all of these email bounces,
should've looked a bit closer at that get_maintainer.pl output...]
I agree that this is a bit more readable, but it evaluates i twice.
That's an easy fix, just do __iter.type instead of (i)->type, but
there's still the possibility of someone passing in something called
__iter as i, and the fix for that tends to be "add more underscores". At
the very least, Al's macro could probably use a comment explaining
what's going on there, though.
--
Omar
On Tue, Mar 17, 2015 at 10:31:51AM +0100, David Sterba wrote:
> Agreed, but the proposed define is rather cryptic and I was not able to
> understand the meaning on the first glance.
>
> > #define iov_iter_rw(i) ((0 ? (struct iov_iter *)0 : (i))->type & RW_MASK)
>
> This worked for me, does not compile with anything else than
> 'struct iov_iter*' as i:
>
> #define iov_iter_rw(i) ({ \
> struct iov_iter __iter = *(i); \
> (i)->type & RW_MASK; \
> })
>
> The assignment is optimized out.
... and you are getting
a) use of rather lousy gccism when plain C would do
b) double evaluation since you've got it wrong (should've been
__iter.type & RW_MASK, if you do it that way). As it is, if argument has
any side effects, your variant will trigger those twice - even if the
destination of the assignment is never used, the side effects remain.
I agree that it could use /* use ?: for typechecking */, but let's not go into
({...}) land unless we absolutely have to.
On Mon, Mar 16, 2015 at 04:33:48AM -0700, Omar Sandoval wrote:
> Hi,
>
> Al, here's some cleanup that you mentioned back in December that I got
> around to (https://lkml.org/lkml/2014/12/15/28).
Applied. See #for-next