[PATCH 00/16] fs: fixup racy check file->f_flags for xxx_write_iter

[PATCH 00/16] fs: fixup racy check file->f_flags for xxx_write_iter

[Dmitry Monakhov]

There are many places inside vfs/fs where code flow depends on file->f_flags,
but this check is racy because one can change it via fcntl(,F_SETFL,)
For example O_DIRECT usually flag checked twice:
  xxx_file_write_iter -> check O_DIRECT, and perform some optimizations
    ->__generic_file_write_iter -> check O_DIRECT,
which may break things: for example http://www.spinics.net/lists/linux-ext4/msg45683.html
For that reason some filesystems simply do not use __generic_file_write_iter()
wihch result in code duplication. Right way to fix this is to save volatile flags
inside kiocb->ki_flags similar to ->ki_pos
Other private discussion: message-id:20141218105101.GD13705@quack.suse.cz

TOC:
##First two patches introduce helpers and update generic code
kiocb_flags-v1/0001-fs-save-file-f_flags-to-kiocb-ki_flags.patch
kiocb_flags-v1/0002-vfs-check-kiocb-ki_flags-instead-filp-fl_flags.patch
## Switch filesystems to kiocb->ki_flags
kiocb_flags-v1/0003-ext4-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0004-9p-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0005-btrfs-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0006-ceph-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0007-cifs-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0008-gfs2-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0009-nfs-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0010-ntfs-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0011-ocfs2-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0012-udf-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0013-xfs-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0014-fuse-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
## Fix pipe and splice issues caused by race with fcntl(,F_SETFL,)
kiocb_flags-v1/0015-pipe-use-is_xxx_kiocb-instead-of-filp-fl_flags.patch
kiocb_flags-v1/0016-splice-fix-race-beween-splice_write-vs-fcntl-F_SETFL.patch

Patch set survived basic run of xfstests

[PATCH 01/16] fs: save file->f_flags to kiocb->ki_flags

[Dmitry Monakhov]

There are many places inside vfs/fs where code flow depends on file->f_flags,
but this check is racy because one can change it via fcntl(,F_SETFL,)
For example O_DIRECT usually flag checked twice:
  xxx_file_write_iter -> check O_DIRECT, and perform some optimization
    ->__generic_file_write_iter -> check O_DIRECT,
which may break things: for example http://www.spinics.net/lists/linux-ext4/msg45683.html
For that reason some filesystems simply do not use __generic_file_write_iter()
which result in code duplication. Right way to fix this is to save volatile flags
inside kiocb->ki_flags similar to ->ki_pos
Other private discussion: message-id:20141218105101.GD13705@quack.suse.cz

This patch store O_DIRECT|O_APPEND|O_NONBLOCK|O_NDELAY
to kiocb->ki_flags on kiocb initialization.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/aio.c           |    7 ++++---
 fs/read_write.c    |   20 ++++++++++++++++++++
 include/linux/fs.h |   30 +++++++++++++++++++++++++++---
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 3b8467a..f58c4d6 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1482,6 +1482,7 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
 			 struct iocb *iocb, bool compat)
 {
 	struct aio_kiocb *req;
+	struct file* filp;
 	ssize_t ret;
 
 	/* enforce forwards compatibility on users */
@@ -1504,14 +1505,14 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
 	if (unlikely(!req))
 		return -EAGAIN;
 
-	req->common.ki_filp = fget(iocb->aio_fildes);
-	if (unlikely(!req->common.ki_filp)) {
+	filp = fget(iocb->aio_fildes);
+	if (unlikely(!filp)) {
 		ret = -EBADF;
 		goto out_put_req;
 	}
+	kiocb_init_file(&req->common, filp);
 	req->common.ki_pos = iocb->aio_offset;
 	req->common.ki_complete = aio_complete;
-	req->common.ki_flags = 0;
 
 	if (iocb->aio_flags & IOCB_FLAG_RESFD) {
 		/*
diff --git a/fs/read_write.c b/fs/read_write.c
index 69128b3..00e1ca4 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -41,6 +41,26 @@ static inline int unsigned_offsets(struct file *file)
 	return file->f_mode & FMODE_UNSIGNED_OFFSET;
 }
 
+void kiocb_init_file(struct kiocb *kiocb, struct file *filp)
+{
+	kiocb->ki_flags = 0;
+	kiocb->ki_filp = filp;
+
+	/* Socket aio */
+	if (kiocb->ki_filp == NULL)
+		return;
+
+	if (filp->f_flags & O_APPEND)
+		kiocb->ki_flags |= IOCB_APPEND;
+	if (filp->f_flags & O_NONBLOCK)
+		kiocb->ki_flags |= IOCB_NONBLOCK;
+	if (filp->f_flags & O_NDELAY)
+		kiocb->ki_flags |= IOCB_NDELAY;
+	if (filp->f_flags & O_DIRECT)
+		kiocb->ki_flags |= IOCB_DIRECT;
+}
+EXPORT_SYMBOL(kiocb_init_file);
+
 /**
  * vfs_setpos - update the file offset for lseek
  * @file:	file structure in question
diff --git a/include/linux/fs.h b/include/linux/fs.h
index dfbd88a..4c20030 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -315,6 +315,10 @@ struct address_space;
 struct writeback_control;
 
 #define IOCB_EVENTFD		(1 << 0)
+#define IOCB_APPEND		(1 << 1)
+#define IOCB_NONBLOCK		(1 << 2)
+#define IOCB_NDELAY		(1 << 3)
+#define IOCB_DIRECT		(1 << 4)
 
 struct kiocb {
 	struct file		*ki_filp;
@@ -329,11 +333,11 @@ static inline bool is_sync_kiocb(struct kiocb *kiocb)
 	return kiocb->ki_complete == NULL;
 }
 
+extern void kiocb_init_file(struct kiocb *kiocb, struct file *filp);
 static inline void init_sync_kiocb(struct kiocb *kiocb, struct file *filp)
 {
-	*kiocb = (struct kiocb) {
-		.ki_filp = filp,
-	};
+	memset(kiocb, 0 , sizeof(*kiocb));
+	kiocb_init_file(kiocb, filp);
 }
 
 /*
@@ -2776,6 +2780,26 @@ extern int generic_show_options(struct seq_file *m, struct dentry *root);
 extern void save_mount_options(struct super_block *sb, char *options);
 extern void replace_mount_options(struct super_block *sb, char *options);
 
+static inline bool is_append_kiocb(struct kiocb *kiocb)
+{
+	return kiocb->ki_flags & IOCB_APPEND;
+}
+
+static inline bool is_direct_kiocb(struct kiocb *kiocb)
+{
+	return (kiocb->ki_flags & IOCB_DIRECT) |
+		IS_DAX(file_inode(kiocb->ki_filp));
+
+}
+
+
+static inline bool is_nonblock_kiocb(struct kiocb *kiocb)
+{
+	return kiocb->ki_flags & IOCB_NONBLOCK;
+}
+
+/* XXX: this is obsolete helper, and will be removed soon.
+ * One should use io_direct_kiocb() instead */
 static inline bool io_is_direct(struct file *filp)
 {
 	return (filp->f_flags & O_DIRECT) || IS_DAX(file_inode(filp));
-- 
1.7.1

[PATCH 02/16] vfs: check kiocb->ki_flags instead filp->fl_flags

[Dmitry Monakhov]

generic_write_checks now accept kiocb as an argument
Unfortunetly it is impossible to get rid of old interface because some crappy
do not support write_iter interface so leave __generic_write_checks as backward
compatibility helper.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 include/linux/fs.h |    8 +++++++-
 mm/filemap.c       |   13 +++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4c20030..992685e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2570,7 +2570,7 @@ extern int sb_min_blocksize(struct super_block *, int);
 
 extern int generic_file_mmap(struct file *, struct vm_area_struct *);
 extern int generic_file_readonly_mmap(struct file *, struct vm_area_struct *);
-int generic_write_checks(struct file *file, loff_t *pos, size_t *count, int isblk);
+int __generic_write_checks(struct file * file, loff_t *pos, size_t *count, int isblk, int append);
 extern ssize_t generic_file_read_iter(struct kiocb *, struct iov_iter *);
 extern ssize_t __generic_file_write_iter(struct kiocb *, struct iov_iter *);
 extern ssize_t generic_file_write_iter(struct kiocb *, struct iov_iter *);
@@ -2798,6 +2798,12 @@ static inline bool is_nonblock_kiocb(struct kiocb *kiocb)
 	return kiocb->ki_flags & IOCB_NONBLOCK;
 }
 
+static inline int generic_write_checks(struct kiocb *iocb, loff_t *pos, size_t *count, int isblk)
+{
+	return __generic_write_checks(iocb->ki_filp, pos, count, isblk,
+				      is_append_kiocb(iocb));
+}
+
 /* XXX: this is obsolete helper, and will be removed soon.
  * One should use io_direct_kiocb() instead */
 static inline bool io_is_direct(struct file *filp)
diff --git a/mm/filemap.c b/mm/filemap.c
index 876f4e6..b519824 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1694,7 +1694,7 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
 	loff_t *ppos = &iocb->ki_pos;
 	loff_t pos = *ppos;
 
-	if (io_is_direct(file)) {
+	if (is_direct_kiocb(iocb)) {
 		struct address_space *mapping = file->f_mapping;
 		struct inode *inode = mapping->host;
 		size_t count = iov_iter_count(iter);
@@ -2260,7 +2260,8 @@ EXPORT_SYMBOL(read_cache_page_gfp);
  * Returns appropriate error code that caller should return or
  * zero in case that write should be allowed.
  */
-inline int generic_write_checks(struct file *file, loff_t *pos, size_t *count, int isblk)
+inline int __generic_write_checks(struct file *file, loff_t *pos, size_t *count,
+				  int isblk, int is_append)
 {
 	struct inode *inode = file->f_mapping->host;
 	unsigned long limit = rlimit(RLIMIT_FSIZE);
@@ -2270,7 +2271,7 @@ inline int generic_write_checks(struct file *file, loff_t *pos, size_t *count, i
 
 	if (!isblk) {
 		/* FIXME: this is for backwards compatibility with 2.4 */
-		if (file->f_flags & O_APPEND)
+		if (is_append)
                         *pos = i_size_read(inode);
 
 		if (limit != RLIM_INFINITY) {
@@ -2333,7 +2334,7 @@ inline int generic_write_checks(struct file *file, loff_t *pos, size_t *count, i
 	}
 	return 0;
 }
-EXPORT_SYMBOL(generic_write_checks);
+EXPORT_SYMBOL(__generic_write_checks);
 
 int pagecache_write_begin(struct file *file, struct address_space *mapping,
 				loff_t pos, unsigned len, unsigned flags,
@@ -2565,7 +2566,7 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	/* We can write back this queue in page reclaim */
 	current->backing_dev_info = inode_to_bdi(inode);
-	err = generic_write_checks(file, &pos, &count, S_ISBLK(inode->i_mode));
+	err = generic_write_checks(iocb, &pos, &count, S_ISBLK(inode->i_mode));
 	if (err)
 		goto out;
 
@@ -2582,7 +2583,7 @@ ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (err)
 		goto out;
 
-	if (io_is_direct(file)) {
+	if (is_direct_kiocb(iocb)) {
 		loff_t endbyte;
 
 		written = generic_file_direct_write(iocb, from, pos);
-- 
1.7.1

[PATCH 03/16] ext4: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: linux-ext4@vger.kernel.org
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ext4/file.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 598abbb..27cf1cc 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -95,7 +95,6 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	struct inode *inode = file_inode(iocb->ki_filp);
 	struct mutex *aio_mutex = NULL;
 	struct blk_plug plug;
-	int o_direct = io_is_direct(file);
 	int overwrite = 0;
 	size_t length = iov_iter_count(from);
 	ssize_t ret;
@@ -105,18 +104,17 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	 * Unaligned direct AIO must be serialized; see comment above
 	 * In the case of O_APPEND, assume that we must always serialize
 	 */
-	if (o_direct &&
+	if (is_direct_kiocb(iocb) &&
 	    ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS) &&
 	    !is_sync_kiocb(iocb) &&
-	    (file->f_flags & O_APPEND ||
-	     ext4_unaligned_aio(inode, from, pos))) {
+	    (is_append_kiocb(iocb) || ext4_unaligned_aio(inode, from, pos))) {
 		aio_mutex = ext4_aio_mutex(inode);
 		mutex_lock(aio_mutex);
 		ext4_unwritten_wait(inode);
 	}
 
 	mutex_lock(&inode->i_mutex);
-	if (file->f_flags & O_APPEND)
+	if (is_append_kiocb(iocb))
 		iocb->ki_pos = pos = i_size_read(inode);
 
 	/*
@@ -138,7 +136,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	}
 
 	iocb->private = &overwrite;
-	if (o_direct) {
+	if (is_direct_kiocb(iocb)) {
 		blk_start_plug(&plug);
 
 
@@ -182,7 +180,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 		if (err < 0)
 			ret = err;
 	}
-	if (o_direct)
+	if (is_direct_kiocb(iocb))
 		blk_finish_plug(&plug);
 
 errout:
-- 
1.7.1

[PATCH 04/16] 9p: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: v9fs-developer@lists.sourceforge.net
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/9p/vfs_file.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c
index b401337..c2a120a 100644
--- a/fs/9p/vfs_file.c
+++ b/fs/9p/vfs_file.c
@@ -509,7 +509,8 @@ v9fs_file_write(struct file *filp, const char __user * data,
 	loff_t origin = *offset;
 
 
-	retval = generic_write_checks(filp, &origin, &count, 0);
+	retval = __generic_write_checks(filp, &origin, &count, 0,
+					filp->f_flags & O_APPEND);
 	if (retval)
 		goto out;
 
-- 
1.7.1

[PATCH 05/16] btrfs: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: linux-btrfs@vger.kernel.org
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/btrfs/file.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index aee18f8..4dc3856 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1747,7 +1747,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 	mutex_lock(&inode->i_mutex);
 
 	current->backing_dev_info = inode_to_bdi(inode);
-	err = generic_write_checks(file, &pos, &count, S_ISBLK(inode->i_mode));
+	err = generic_write_checks(iocb, &pos, &count, S_ISBLK(inode->i_mode));
 	if (err) {
 		mutex_unlock(&inode->i_mutex);
 		goto out;
@@ -1800,7 +1800,7 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
 	if (sync)
 		atomic_inc(&BTRFS_I(inode)->sync_writers);
 
-	if (file->f_flags & O_DIRECT) {
+	if (is_direct_kiocb(iocb)) {
 		num_written = __btrfs_direct_write(iocb, from, pos);
 	} else {
 		num_written = __btrfs_buffered_write(file, from, pos);
-- 
1.7.1

[PATCH 06/16] ceph: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: ceph-devel@vger.kernel.org
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ceph/file.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 139f2fe..3ac67bf 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -442,7 +442,7 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *i,
 
 	dout("sync_read on file %p %llu~%u %s\n", file, off,
 	     (unsigned)len,
-	     (file->f_flags & O_DIRECT) ? "O_DIRECT" : "");
+	     (is_direct_kiocb(iocb)) ? "O_DIRECT" : "");
 
 	if (!len)
 		return 0;
@@ -457,7 +457,7 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *i,
 	if (ret < 0)
 		return ret;
 
-	if (file->f_flags & O_DIRECT) {
+	if (is_direct_kiocb(iocb)) {
 		while (iov_iter_count(i)) {
 			size_t start;
 			ssize_t n;
@@ -828,8 +828,7 @@ again:
 		return ret;
 
 	if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 ||
-	    (iocb->ki_filp->f_flags & O_DIRECT) ||
-	    (fi->flags & CEPH_F_SYNC)) {
+	    is_direct_kiocb(iocb) || (fi->flags & CEPH_F_SYNC)) {
 
 		dout("aio_sync_read %p %llx.%llx %llu~%u got cap refs on %s\n",
 		     inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len,
@@ -953,7 +952,7 @@ static ssize_t ceph_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	/* We can write back this queue in page reclaim */
 	current->backing_dev_info = inode_to_bdi(inode);
 
-	err = generic_write_checks(file, &pos, &count, S_ISBLK(inode->i_mode));
+	err = generic_write_checks(iocb, &pos, &count, S_ISBLK(inode->i_mode));
 	if (err)
 		goto out;
 
@@ -997,12 +996,12 @@ retry_snap:
 	     inode, ceph_vinop(inode), pos, count, ceph_cap_string(got));
 
 	if ((got & (CEPH_CAP_FILE_BUFFER|CEPH_CAP_FILE_LAZYIO)) == 0 ||
-	    (file->f_flags & O_DIRECT) || (fi->flags & CEPH_F_SYNC)) {
+	    is_direct_kiocb(iocb) || (fi->flags & CEPH_F_SYNC)) {
 		struct iov_iter data;
 		mutex_unlock(&inode->i_mutex);
 		/* we might need to revert back to that point */
 		data = *from;
-		if (file->f_flags & O_DIRECT)
+		if (is_direct_kiocb(iocb))
 			written = ceph_sync_direct_write(iocb, &data, pos);
 		else
 			written = ceph_sync_write(iocb, &data, pos);
-- 
1.7.1

[PATCH 07/16] cifs: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: linux-cifs@vger.kernel.org
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/cifs/file.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a94b3e6..25527e9 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2560,7 +2560,7 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from,
 }
 
 static ssize_t
-cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
+cifs_iovec_write(struct kiocb *iocb, struct iov_iter *from, loff_t *poffset)
 {
 	size_t len;
 	ssize_t total_written = 0;
@@ -2573,7 +2573,7 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 	int rc;
 
 	len = iov_iter_count(from);
-	rc = generic_write_checks(file, poffset, &len, 0);
+	rc = generic_write_checks(iocb, poffset, &len, 0);
 	if (rc)
 		return rc;
 
@@ -2583,8 +2583,8 @@ cifs_iovec_write(struct file *file, struct iov_iter *from, loff_t *poffset)
 	iov_iter_truncate(from, len);
 
 	INIT_LIST_HEAD(&wdata_list);
-	cifs_sb = CIFS_FILE_SB(file);
-	open_file = file->private_data;
+	cifs_sb = CIFS_FILE_SB(iocb->ki_filp);
+	open_file = iocb->ki_filp->private_data;
 	tcon = tlink_tcon(open_file->tlink);
 
 	if (!tcon->ses->server->ops->async_writev)
@@ -2670,7 +2670,7 @@ ssize_t cifs_user_writev(struct kiocb *iocb, struct iov_iter *from)
 	 * write request.
 	 */
 
-	written = cifs_iovec_write(iocb->ki_filp, from, &pos);
+	written = cifs_iovec_write(iocb, from, &pos);
 	if (written > 0) {
 		set_bit(CIFS_INO_INVALID_MAPPING, &CIFS_I(inode)->flags);
 		iocb->ki_pos = pos;
@@ -2696,7 +2696,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
 	 */
 	down_read(&cinode->lock_sem);
 	mutex_lock(&inode->i_mutex);
-	if (file->f_flags & O_APPEND)
+	if (is_append_kiocb(iocb))
 		lock_pos = i_size_read(inode);
 	if (!cifs_find_lock_conflict(cfile, lock_pos, iov_iter_count(from),
 				     server->vals->exclusive_lock_type, NULL,
-- 
1.7.1

[PATCH 08/16] gfs2: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: cluster-devel@redhat.com
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/gfs2/file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index f6fc412..25da110 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -709,7 +709,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	gfs2_size_hint(file, iocb->ki_pos, iov_iter_count(from));
 
-	if (file->f_flags & O_APPEND) {
+	if (is_append_kiocb(iocb)) {
 		struct gfs2_holder gh;
 
 		ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
-- 
1.7.1

[Cluster-devel] [PATCH 08/16] gfs2: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: cluster-devel at redhat.com
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/gfs2/file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index f6fc412..25da110 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -709,7 +709,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 
 	gfs2_size_hint(file, iocb->ki_pos, iov_iter_count(from));
 
-	if (file->f_flags & O_APPEND) {
+	if (is_append_kiocb(iocb)) {
 		struct gfs2_holder gh;
 
 		ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);
-- 
1.7.1

[PATCH 09/16] nfs: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: linux-nfs@vger.kernel.org
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/nfs/direct.c |    7 +++----
 fs/nfs/file.c   |    6 +++---
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index c3929fb..76950c6 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -964,8 +964,7 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, struct iov_iter *iter,
 				loff_t pos)
 {
 	ssize_t result = -EINVAL;
-	struct file *file = iocb->ki_filp;
-	struct address_space *mapping = file->f_mapping;
+	struct address_space *mapping = iocb->ki_filp->f_mapping;
 	struct inode *inode = mapping->host;
 	struct nfs_direct_req *dreq;
 	struct nfs_lock_context *l_ctx;
@@ -976,9 +975,9 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, struct iov_iter *iter,
 	nfs_add_stats(mapping->host, NFSIOS_DIRECTWRITTENBYTES, count);
 
 	dfprintk(FILE, "NFS: direct write(%pD2, %zd@%Ld)\n",
-		file, count, (long long) pos);
+		iocb->ki_filp, count, (long long) pos);
 
-	result = generic_write_checks(file, &pos, &count, 0);
+	result = generic_write_checks(iocb, &pos, &count, 0);
 	if (result)
 		goto out;
 
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 37b1558..709cb7f 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -170,7 +170,7 @@ nfs_file_read(struct kiocb *iocb, struct iov_iter *to)
 	struct inode *inode = file_inode(iocb->ki_filp);
 	ssize_t result;
 
-	if (iocb->ki_filp->f_flags & O_DIRECT)
+	if (is_direct_kiocb(iocb))
 		return nfs_file_direct_read(iocb, to, iocb->ki_pos);
 
 	dprintk("NFS: read(%pD2, %zu@%lu)\n",
@@ -680,7 +680,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from)
 	if (result)
 		return result;
 
-	if (file->f_flags & O_DIRECT)
+	if (is_direct_kiocb(iocb))
 		return nfs_file_direct_write(iocb, from, pos);
 
 	dprintk("NFS: write(%pD2, %zu@%Ld)\n",
@@ -692,7 +692,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from)
 	/*
 	 * O_APPEND implies that we must revalidate the file length.
 	 */
-	if (file->f_flags & O_APPEND) {
+	if (is_append_kiocb(iocb)) {
 		result = nfs_revalidate_file_size(inode, file);
 		if (result)
 			goto out;
-- 
1.7.1

[PATCH 10/16] ntfs: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: linux-ntfs-dev@lists.sourceforge.net
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ntfs/file.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index c1da78d..648b3a4 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -345,7 +345,8 @@ static ssize_t ntfs_prepare_file_for_write(struct file *file, loff_t *ppos,
 			(unsigned long long)*ppos, (unsigned long)*count);
 	/* We can write back this queue in page reclaim. */
 	current->backing_dev_info = inode_to_bdi(vi);
-	err = generic_write_checks(file, ppos, count, S_ISBLK(vi->i_mode));
+	err = __generic_write_checks(file, ppos, count, S_ISBLK(vi->i_mode),
+				     file->f_flags & O_APPEND);
 	if (unlikely(err))
 		goto out;
 	/*
-- 
1.7.1

[PATCH 11/16] ocfs2: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: ocfs2-devel@oss.oracle.com
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ocfs2/file.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 266845d..c22b240 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2283,8 +2283,8 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
 	if (count == 0)
 		return 0;
 
-	appending = file->f_flags & O_APPEND ? 1 : 0;
-	direct_io = file->f_flags & O_DIRECT ? 1 : 0;
+	appending = is_append_kiocb(iocb);
+	direct_io = is_direct_kiocb(iocb);
 
 	mutex_lock(&inode->i_mutex);
 
@@ -2374,7 +2374,7 @@ relock:
 	/* communicate with ocfs2_dio_end_io */
 	ocfs2_iocb_set_rw_locked(iocb, rw_level);
 
-	ret = generic_write_checks(file, ppos, &count,
+	ret = generic_write_checks(iocb, ppos, &count,
 				   S_ISBLK(inode->i_mode));
 	if (ret)
 		goto out_dio;
@@ -2436,7 +2436,7 @@ relock:
 
 out_dio:
 	/* buffered aio wouldn't have proper lock coverage today */
-	BUG_ON(ret == -EIOCBQUEUED && !(file->f_flags & O_DIRECT));
+	BUG_ON(ret == -EIOCBQUEUED && !is_direct_kiocb(iocb));
 
 	if (((file->f_flags & O_DSYNC) && !direct_io) || IS_SYNC(inode) ||
 	    ((file->f_flags & O_DIRECT) && !direct_io)) {
@@ -2547,7 +2547,7 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
 	 * buffered reads protect themselves in ->readpage().  O_DIRECT reads
 	 * need locks to protect pending reads from racing with truncate.
 	 */
-	if (filp->f_flags & O_DIRECT) {
+	if (is_direct_kiocb(iocb)) {
 		have_alloc_sem = 1;
 		ocfs2_iocb_set_sem_locked(iocb);
 
@@ -2581,7 +2581,7 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
 	trace_generic_file_aio_read_ret(ret);
 
 	/* buffered aio wouldn't have proper lock coverage today */
-	BUG_ON(ret == -EIOCBQUEUED && !(filp->f_flags & O_DIRECT));
+	BUG_ON(ret == -EIOCBQUEUED && !is_direct_kiocb(iocb));
 
 	/* see ocfs2_file_write_iter */
 	if (ret == -EIOCBQUEUED || !ocfs2_iocb_is_rw_locked(iocb)) {
-- 
1.7.1

[PATCH 12/16] udf: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: jack@suse.cz
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/udf/file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/udf/file.c b/fs/udf/file.c
index 7f885cc..78ccc0f 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -128,7 +128,7 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	mutex_lock(&inode->i_mutex);
 	down_write(&iinfo->i_data_sem);
 	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
-		if (file->f_flags & O_APPEND)
+		if (is_append_kiocb(iocb))
 			pos = inode->i_size;
 		else
 			pos = iocb->ki_pos;
-- 
1.7.1

[PATCH 13/16] xfs: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Also function interface cleanup in order to vfs:write_iter interface agreeament

Cc: xfs@oss.sgi.com
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/xfs/xfs_file.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index f44212f..148039b 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -279,7 +279,7 @@ xfs_file_read_iter(
 
 	XFS_STATS_INC(xs_read_calls);
 
-	if (unlikely(file->f_flags & O_DIRECT))
+	if (unlikely(is_direct_kiocb(iocb)))
 		ioflags |= XFS_IO_ISDIRECT;
 	if (file->f_mode & FMODE_NOCMTIME)
 		ioflags |= XFS_IO_INVIS;
@@ -544,17 +544,18 @@ xfs_zero_eof(
  */
 STATIC ssize_t
 xfs_file_aio_write_checks(
-	struct file		*file,
+	struct kiocb		*iocb,
 	loff_t			*pos,
 	size_t			*count,
 	int			*iolock)
 {
+	struct file		*file = iocb->ki_filp;
 	struct inode		*inode = file->f_mapping->host;
 	struct xfs_inode	*ip = XFS_I(inode);
 	int			error = 0;
 
 restart:
-	error = generic_write_checks(file, pos, count, S_ISBLK(inode->i_mode));
+	error = generic_write_checks(iocb, pos, count, S_ISBLK(inode->i_mode));
 	if (error)
 		return error;
 
@@ -678,7 +679,7 @@ xfs_file_dio_aio_write(
 		xfs_rw_ilock(ip, iolock);
 	}
 
-	ret = xfs_file_aio_write_checks(file, &pos, &count, &iolock);
+	ret = xfs_file_aio_write_checks(iocb, &pos, &count, &iolock);
 	if (ret)
 		goto out;
 	iov_iter_truncate(from, count);
@@ -739,7 +740,7 @@ xfs_file_buffered_aio_write(
 
 	xfs_rw_ilock(ip, iolock);
 
-	ret = xfs_file_aio_write_checks(file, &pos, &count, &iolock);
+	ret = xfs_file_aio_write_checks(iocb, &pos, &count, &iolock);
 	if (ret)
 		goto out;
 
@@ -803,7 +804,7 @@ xfs_file_write_iter(
 	if (XFS_FORCED_SHUTDOWN(ip->i_mount))
 		return -EIO;
 
-	if (unlikely(file->f_flags & O_DIRECT))
+	if (unlikely(is_direct_kiocb(iocb)))
 		ret = xfs_file_dio_aio_write(iocb, from);
 	else
 		ret = xfs_file_buffered_aio_write(iocb, from);
-- 
1.7.1

[PATCH 14/16] fuse: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Cc: fuse-devel@lists.sourceforge.net
Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/fuse/file.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index ff102cb..ba8ad87 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1167,7 +1167,7 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	/* We can write back this queue in page reclaim */
 	current->backing_dev_info = inode_to_bdi(inode);
 
-	err = generic_write_checks(file, &pos, &count, S_ISBLK(inode->i_mode));
+	err = generic_write_checks(iocb, &pos, &count, S_ISBLK(inode->i_mode));
 	if (err)
 		goto out;
 
@@ -1183,7 +1183,7 @@ static ssize_t fuse_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	if (err)
 		goto out;
 
-	if (file->f_flags & O_DIRECT) {
+	if (is_direct_kiocb(iocb)) {
 		written = generic_file_direct_write(iocb, from, pos);
 		if (written < 0 || !iov_iter_count(from))
 			goto out;
@@ -1421,7 +1421,8 @@ static ssize_t __fuse_direct_write(struct fuse_io_priv *io,
 	ssize_t res;
 
 
-	res = generic_write_checks(file, ppos, &count, 0);
+	res = __generic_write_checks(file, ppos, &count, 0,
+				     file->f_flags & O_APPEND);
 	if (!res) {
 		iov_iter_truncate(iter, count);
 		res = fuse_direct_io(io, iter, ppos, FUSE_DIO_WRITE);
-- 
1.7.1

[PATCH 15/16] pipe: use is_xxx_kiocb instead of filp->fl_flags

[Dmitry Monakhov]

Also fix other long standing issues caused by fcntl(,F_SETFL,):
- One can disable O_DIRECT for pipe[1] (paketized IO), but can not enable it again.
- Currently we do not set O_APPEND on pipe[1] (IMHO it is wrong, but let it be)
  so it is reasonable to completely prohibit change O_APPEND flag on both
  end's of pipe. Add ->check_flags method in order to diallow O_APPEND toggling.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/fcntl.c |    6 ++++--
 fs/pipe.c  |   20 +++++++++++++++-----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index ee85cd4..0bdc9c7 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -51,9 +51,11 @@ static int setfl(int fd, struct file * filp, unsigned long arg)
 	       if (arg & O_NDELAY)
 		   arg |= O_NONBLOCK;
 
+	/* allowed only for inodes with ->direct_io method or write pipe */
 	if (arg & O_DIRECT) {
-		if (!filp->f_mapping || !filp->f_mapping->a_ops ||
-			!filp->f_mapping->a_ops->direct_IO)
+		if ((!filp->f_mapping || !filp->f_mapping->a_ops ||
+		     !filp->f_mapping->a_ops->direct_IO) &&
+		    !(get_pipe_info(filp) && (filp->f_flags | O_WRONLY)))
 				return -EINVAL;
 	}
 
diff --git a/fs/pipe.c b/fs/pipe.c
index 2d084f2..95b5fe4 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -301,7 +301,7 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
 			 */
 			if (ret)
 				break;
-			if (filp->f_flags & O_NONBLOCK) {
+			if (is_nonblock_kiocb(iocb)) {
 				ret = -EAGAIN;
 				break;
 			}
@@ -329,9 +329,9 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
 	return ret;
 }
 
-static inline int is_packetized(struct file *file)
+static inline int is_packetized(struct kiocb *kiocb)
 {
-	return (file->f_flags & O_DIRECT) != 0;
+	return is_direct_kiocb(kiocb);
 }
 
 static ssize_t
@@ -427,7 +427,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 			buf->offset = 0;
 			buf->len = copied;
 			buf->flags = 0;
-			if (is_packetized(filp)) {
+			if (is_packetized(iocb)) {
 				buf->ops = &packet_pipe_buf_ops;
 				buf->flags = PIPE_BUF_FLAG_PACKET;
 			}
@@ -439,7 +439,7 @@ pipe_write(struct kiocb *iocb, struct iov_iter *from)
 		}
 		if (bufs < pipe->buffers)
 			continue;
-		if (filp->f_flags & O_NONBLOCK) {
+		if (is_nonblock_kiocb(iocb)) {
 			if (!ret)
 				ret = -EAGAIN;
 			break;
@@ -943,6 +943,15 @@ err:
 	return ret;
 }
 
+/* XXX: Currently it is not possible distinguish read side from write one */
+static int pipe_check_flags(int flags)
+{
+	if (flags & O_APPEND)
+	    return -EINVAL;
+
+	return 0;
+}
+
 const struct file_operations pipefifo_fops = {
 	.open		= fifo_open,
 	.llseek		= no_llseek,
@@ -954,6 +963,7 @@ const struct file_operations pipefifo_fops = {
 	.unlocked_ioctl	= pipe_ioctl,
 	.release	= pipe_release,
 	.fasync		= pipe_fasync,
+	.check_flags	= pipe_check_flags,
 };
 
 /*
-- 
1.7.1

[PATCH 16/16] splice: fix race beween splice_write vs fcntl(,F_SETFL,)

[Dmitry Monakhov]

file->f_flags & O_APPEND is checked twice
-> do_splice_direct or do_splice: return EINVAL if O_APPEND enabled
   -> generic_write_checks: seek to end in case of O_APPEND
This is obviously whong and result in unpredictable behaviour if raced with
fcntl. It is reasonable to recheck append flag after kiocb was constructed (
f_flags becomes stable), for that reason we should use special analog of
vfs_write_iter()

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/splice.c |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index 41cbb16..7ac43db 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -922,6 +922,27 @@ ssize_t splice_from_pipe(struct pipe_inode_info *pipe, struct file *out,
 
 	return ret;
 }
+ssize_t splice_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos)
+{
+	struct kiocb kiocb;
+	ssize_t ret;
+
+	if (!file->f_op->write_iter)
+		return -EINVAL;
+
+	init_sync_kiocb(&kiocb, file);
+	if (is_append_kiocb(&kiocb))
+		return -EINVAL;
+
+	kiocb.ki_pos = *ppos;
+	iter->type |= WRITE;
+	ret = file->f_op->write_iter(&kiocb, iter);
+	BUG_ON(ret == -EIOCBQUEUED);
+	if (ret > 0)
+		*ppos = kiocb.ki_pos;
+	return ret;
+}
+
 
 /**
  * iter_file_splice_write - splice data from a pipe to a file
@@ -1005,7 +1026,7 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
 
 		iov_iter_bvec(&from, ITER_BVEC | WRITE, array, n,
 			      sd.total_len - left);
-		ret = vfs_iter_write(out, &from, &sd.pos);
+		ret = splice_iter_write(out, &from, &sd.pos);
 		if (ret <= 0)
 			break;
 
-- 
1.7.1

Re: [PATCH 02/16] vfs: check kiocb->ki_flags instead filp->fl_flags

[Al Viro]

On Sat, Apr 04, 2015 at 11:13:11PM +0400, Dmitry Monakhov wrote:
> generic_write_checks now accept kiocb as an argument
> Unfortunetly it is impossible to get rid of old interface because some crappy
> do not support write_iter interface so leave __generic_write_checks as backward
> compatibility helper.

Check the current vfs.git#for-next (there's even some generic_write_checks()
work in it).  The same goes for the rest of the series.  Please, rebase it.

What's more, generic_write_checks() should take iov_iter *, not the address
of something its ->count had been copied into.  Note that all callers of that
thing end up doing iov_iter_truncate() pretty soon afterwards.  I hadn't
pushed that one out yet (there is some weirdness in ocfs2 which might be
a bug; I want to sort that out first), but that's where it's going.

Re: [PATCH 02/16] vfs: check kiocb->ki_flags instead filp->fl_flags

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Sat, Apr 04, 2015 at 11:13:11PM +0400, Dmitry Monakhov wrote:
>> generic_write_checks now accept kiocb as an argument
>> Unfortunetly it is impossible to get rid of old interface because some crappy
>> do not support write_iter interface so leave __generic_write_checks as backward
>> compatibility helper.
>
> Check the current vfs.git#for-next (there's even some generic_write_checks()
> work in it).  The same goes for the rest of the series.  Please, rebase it.
Ok. Yes you right. I've prepared patches against vfs.git#for-next(48a0c62) but
it is too old. Will rebase. Also it is reasonable to fold my fs-xxx
conversion to one combo patch similar to (d04cfe7840)
>
> What's more, generic_write_checks() should take iov_iter *, not the address
> of something its ->count had been copied into.  Note that all callers of that
> thing end up doing iov_iter_truncate() pretty soon afterwards.  I hadn't
> pushed that one out yet (there is some weirdness in ocfs2 which might be
> a bug; I want to sort that out first), but that's where it's going.
I'm not sure I have get your point about ocfs2 because it does
iov_iter_truncate() right after generic_write_checks()
But nfs definitely has a bug because iter was not truncated after generic_write_checks
->nfs_file_direct_write
  ->generic_write_checks(file, &pos, &count);
  dreq->bytes_left = count;
  task_io_account_write(count);
  ->nfs_direct_write_schedule_iovec(dreq, iter, pos);
    ###Ignore dreq->bytes_left(ala count) and submit non truncated iter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH 02/16] vfs: check kiocb->ki_flags instead filp->fl_flags

[Al Viro]

On Sun, Apr 05, 2015 at 02:03:22PM +0300, Dmitry Monakhov wrote:

> I'm not sure I have get your point about ocfs2 because it does
> iov_iter_truncate() right after generic_write_checks()

This
        ret = ocfs2_prepare_inode_for_write(file, ppos, count, appending,
                                            &can_do_direct, &has_refcount);
being done before generic_write_checks().  It actually duplicates some
parts of generic_write_checks() inside (O_APPEND-related, and AFAICS
they _are_ triggered twice that way).

Re: [PATCH 02/16] vfs: check kiocb->ki_flags instead filp->fl_flags

[Al Viro]

On Sun, Apr 05, 2015 at 07:11:45PM +0100, Al Viro wrote:
> On Sun, Apr 05, 2015 at 02:03:22PM +0300, Dmitry Monakhov wrote:
> 
> > I'm not sure I have get your point about ocfs2 because it does
> > iov_iter_truncate() right after generic_write_checks()
> 
> This
>         ret = ocfs2_prepare_inode_for_write(file, ppos, count, appending,
>                                             &can_do_direct, &has_refcount);
> being done before generic_write_checks().  It actually duplicates some
> parts of generic_write_checks() inside (O_APPEND-related, and AFAICS
> they _are_ triggered twice that way).

XFS seems to be buggered as well:
        /* DIO must be aligned to device logical sector size */
        if ((pos | count) & target->bt_logical_sectormask)
                return -EINVAL;

        /* "unaligned" here means not aligned to a filesystem block */
        if ((pos & mp->m_blockmask) || ((pos + count) & mp->m_blockmask))
                unaligned_io = 1;
...
        ret = xfs_file_aio_write_checks(file, &pos, &count, &iolock);

now, play with rlimit() and suddenly the alignment checks above have nothing
to do with what'll actually happen after that sucker - it's calling
generic_write_checks(), so...

Incidentally, we want the result of alignment check to decide how to take
the lock that protects the file size, so simply lifting O_APPEND treatment
above those won't do.  I suspect that in case of lock taken shared we
need to redo alignment checks and treat "it became unaligned" as "unlock
and redo it with lock taken exclusive".

BTW, xfs_break_layouts() having dropped and regained lock would invalidate
the O_APPEND treatment in generic_write_checks() just prior (both in
xfs_file_aio_write_checks())...

Al "really not fond of xfs_rw_ilock()" Viro...

Re: [Cluster-devel] [PATCH 08/16] gfs2: use is_xxx_kiocb instead of filp->fl_flags

[Steven Whitehouse]

Hi,

On 04/04/15 20:13, Dmitry Monakhov wrote:
> Cc: cluster-devel@redhat.com
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.

> ---
>   fs/gfs2/file.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index f6fc412..25da110 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -709,7 +709,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   
>   	gfs2_size_hint(file, iocb->ki_pos, iov_iter_count(from));
>   
> -	if (file->f_flags & O_APPEND) {
> +	if (is_append_kiocb(iocb)) {
>   		struct gfs2_holder gh;
>   
>   		ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);

[Cluster-devel] [PATCH 08/16] gfs2: use is_xxx_kiocb instead of filp->fl_flags

[Steven Whitehouse]

Hi,

On 04/04/15 20:13, Dmitry Monakhov wrote:
> Cc: cluster-devel at redhat.com
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
Acked-by: Steven Whitehouse <swhiteho@redhat.com>

Steve.

> ---
>   fs/gfs2/file.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index f6fc412..25da110 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -709,7 +709,7 @@ static ssize_t gfs2_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   
>   	gfs2_size_hint(file, iocb->ki_pos, iov_iter_count(from));
>   
> -	if (file->f_flags & O_APPEND) {
> +	if (is_append_kiocb(iocb)) {
>   		struct gfs2_holder gh;
>   
>   		ret = gfs2_glock_nq_init(ip->i_gl, LM_ST_SHARED, 0, &gh);