* [PATCH v5 4/7] vfs: RWF_NONBLOCK flag for preadv2
[not found] <cover.1415220890.git.milosz@adfin.com>
@ 2014-11-05 21:14 ` Milosz Tanski
2014-11-10 16:07 ` Sage Weil
2014-11-05 21:14 ` [PATCH v5 6/7] fs: pass iocb to generic_write_sync Milosz Tanski
1 sibling, 1 reply; 7+ messages in thread
From: Milosz Tanski @ 2014-11-05 21:14 UTC (permalink / raw)
To: linux-kernel
Cc: Christoph Hellwig, linux-fsdevel, linux-aio, Mel Gorman,
Volker Lendecke, Tejun Heo, Jeff Moyer, Theodore Ts'o,
Al Viro, linux-api, Michael Kerrisk, linux-arch, ceph-devel,
linux-cifs, samba-technical, linux-nfs, linux-xfs, ocfs2-devel,
linux-mm
generic_file_read_iter() supports a new flag RWF_NONBLOCK which says that we
only want to read the data if it's already in the page cache.
Additionally, there are a few filesystems that we have to specifically
bail early if RWF_NONBLOCK because the op would block. Christoph Hellwig
contributed this code.
Signed-off-by: Milosz Tanski <milosz@adfin.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
---
fs/ceph/file.c | 2 ++
fs/cifs/file.c | 6 ++++++
fs/nfs/file.c | 5 ++++-
fs/ocfs2/file.c | 6 ++++++
fs/pipe.c | 3 ++-
fs/read_write.c | 38 +++++++++++++++++++++++++-------------
fs/xfs/xfs_file.c | 4 ++++
include/linux/fs.h | 3 +++
mm/filemap.c | 18 ++++++++++++++++++
mm/shmem.c | 4 ++++
10 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index d7e0da8..b798b5c 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -822,6 +822,8 @@ again:
if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 ||
(iocb->ki_filp->f_flags & O_DIRECT) ||
(fi->flags & CEPH_F_SYNC)) {
+ if (iocb->ki_rwflags & O_NONBLOCK)
+ return -EAGAIN;
dout("aio_sync_read %p %llx.%llx %llu~%u got cap refs on %s\n",
inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len,
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 3e4d00a..c485afa 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3005,6 +3005,9 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
struct cifs_readdata *rdata, *tmp;
struct list_head rdata_list;
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
+
len = iov_iter_count(to);
if (!len)
return 0;
@@ -3123,6 +3126,9 @@ cifs_strict_readv(struct kiocb *iocb, struct iov_iter *to)
((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0))
return generic_file_read_iter(iocb, to);
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
+
/*
* We need to hold the sem to be sure nobody modifies lock list
* with a brlock that prevents reading.
diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 2ab6f00..aa9046f 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -171,8 +171,11 @@ 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 (iocb->ki_filp->f_flags & O_DIRECT) {
+ if (iocb->ki_rwflags & O_NONBLOCK)
+ return -EAGAIN;
return nfs_file_direct_read(iocb, to, iocb->ki_pos);
+ }
dprintk("NFS: read(%pD2, %zu@%lu)\n",
iocb->ki_filp,
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 324dc93..bb66ca4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2472,6 +2472,12 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
filp->f_path.dentry->d_name.name,
to->nr_segs); /* GRRRRR */
+ /*
+ * No non-blocking reads for ocfs2 for now. Might be doable with
+ * non-blocking cluster lock helpers.
+ */
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
if (!inode) {
ret = -EINVAL;
diff --git a/fs/pipe.c b/fs/pipe.c
index 21981e5..212bf68 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -302,7 +302,8 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
*/
if (ret)
break;
- if (filp->f_flags & O_NONBLOCK) {
+ if ((filp->f_flags & O_NONBLOCK) ||
+ (iocb->ki_rwflags & RWF_NONBLOCK)) {
ret = -EAGAIN;
break;
}
diff --git a/fs/read_write.c b/fs/read_write.c
index 907735c..cba7d4c 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -835,14 +835,19 @@ static ssize_t do_readv_writev(int type, struct file *file,
file_start_write(file);
}
- if (iter_fn)
+ if (iter_fn) {
ret = do_iter_readv_writev(file, type, iov, nr_segs, tot_len,
pos, iter_fn, flags);
- else if (fnv)
- ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
- pos, fnv);
- else
- ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
+ } else {
+ if (type == READ && (flags & RWF_NONBLOCK))
+ return -EAGAIN;
+
+ if (fnv)
+ ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
+ pos, fnv);
+ else
+ ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
+ }
if (type != READ)
file_end_write(file);
@@ -866,8 +871,10 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
return -EBADF;
if (!(file->f_mode & FMODE_CAN_READ))
return -EINVAL;
- if (flags & ~0)
+ if (flags & ~RWF_NONBLOCK)
return -EINVAL;
+ if ((file->f_flags & O_DIRECT) && (flags & RWF_NONBLOCK))
+ return -EAGAIN;
return do_readv_writev(READ, file, vec, vlen, pos, flags);
}
@@ -1069,14 +1076,19 @@ static ssize_t compat_do_readv_writev(int type, struct file *file,
file_start_write(file);
}
- if (iter_fn)
+ if (iter_fn) {
ret = do_iter_readv_writev(file, type, iov, nr_segs, tot_len,
pos, iter_fn, flags);
- else if (fnv)
- ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
- pos, fnv);
- else
- ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
+ } else {
+ if (type == READ && (flags & RWF_NONBLOCK))
+ return -EAGAIN;
+
+ if (fnv)
+ ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
+ pos, fnv);
+ else
+ ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
+ }
if (type != READ)
file_end_write(file);
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index eb596b4..b1f6334 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -246,6 +246,10 @@ xfs_file_read_iter(
XFS_STATS_INC(xs_read_calls);
+ /* XXX: need a non-blocking iolock helper, shouldn't be too hard */
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
+
if (unlikely(file->f_flags & O_DIRECT))
ioflags |= XFS_IO_ISDIRECT;
if (file->f_mode & FMODE_NOCMTIME)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9ed5711..eaebd99 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1459,6 +1459,9 @@ struct block_device_operations;
#define HAVE_COMPAT_IOCTL 1
#define HAVE_UNLOCKED_IOCTL 1
+/* These flags are used for the readv/writev syscalls with flags. */
+#define RWF_NONBLOCK 0x00000001
+
struct iov_iter;
struct file_operations {
diff --git a/mm/filemap.c b/mm/filemap.c
index 530c263..09d3af3 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1494,6 +1494,8 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
find_page:
page = find_get_page(mapping, index);
if (!page) {
+ if (flags & RWF_NONBLOCK)
+ goto would_block;
page_cache_sync_readahead(mapping,
ra, filp,
index, last_index - index);
@@ -1585,6 +1587,11 @@ page_ok:
continue;
page_not_up_to_date:
+ if (flags & RWF_NONBLOCK) {
+ page_cache_release(page);
+ goto would_block;
+ }
+
/* Get exclusive access to the page ... */
error = lock_page_killable(page);
if (unlikely(error))
@@ -1604,6 +1611,12 @@ page_not_up_to_date_locked:
goto page_ok;
}
+ if (flags & RWF_NONBLOCK) {
+ unlock_page(page);
+ page_cache_release(page);
+ goto would_block;
+ }
+
readpage:
/*
* A previous I/O error may have been due to temporary
@@ -1674,6 +1687,8 @@ no_cached_page:
goto readpage;
}
+would_block:
+ error = -EAGAIN;
out:
ra->prev_pos = prev_index;
ra->prev_pos <<= PAGE_CACHE_SHIFT;
@@ -1707,6 +1722,9 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
size_t count = iov_iter_count(iter);
loff_t size;
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
+
if (!count)
goto out; /* skip atime */
size = i_size_read(inode);
diff --git a/mm/shmem.c b/mm/shmem.c
index cd6fc75..5c30f04 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1531,6 +1531,10 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
ssize_t retval = 0;
loff_t *ppos = &iocb->ki_pos;
+ /* XXX: should be easily supportable */
+ if (iocb->ki_rwflags & RWF_NONBLOCK)
+ return -EAGAIN;
+
/*
* Might this read be for a stacking filesystem? Then when reading
* holes of a sparse file, we actually need to allocate those pages,
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 6/7] fs: pass iocb to generic_write_sync
[not found] <cover.1415220890.git.milosz@adfin.com>
2014-11-05 21:14 ` [PATCH v5 4/7] vfs: RWF_NONBLOCK flag for preadv2 Milosz Tanski
@ 2014-11-05 21:14 ` Milosz Tanski
2014-11-06 10:18 ` [Cluster-devel] " Steven Whitehouse
` (2 more replies)
1 sibling, 3 replies; 7+ messages in thread
From: Milosz Tanski @ 2014-11-05 21:14 UTC (permalink / raw)
To: linux-kernel
Cc: Christoph Hellwig, Christoph Hellwig, linux-fsdevel, linux-aio,
Mel Gorman, Volker Lendecke, Tejun Heo, Jeff Moyer,
Theodore Ts'o, Al Viro, linux-api, Michael Kerrisk,
linux-arch, linux-btrfs, linux-cifs, linux-ext4, linux-ntfs-dev,
linux-xfs, cluster-devel
From: Christoph Hellwig <hch@lst.de>
Clean up the generic_write_sync by just passing an iocb and a bytes
written / negative errno argument. In addition to simplifying the
callers this also prepares for passing a per-operation O_DSYNC
flag. Two callers didn't quite fit that scheme:
- dio_complete didn't both to update ki_pos as we don't need it
on a iocb that is about to be freed, so we had to add it. Additionally
it also synced out written data in the error case, which has been
changed to operate like the other callers.
- gfs2 also used generic_write_sync to implement a crude version
of fallocate. It has been switched to use an open coded variant
instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/block_dev.c | 8 +-------
fs/btrfs/file.c | 7 ++-----
fs/cifs/file.c | 8 +-------
fs/direct-io.c | 8 ++------
fs/ext4/file.c | 8 +-------
fs/gfs2/file.c | 9 +++++++--
fs/ntfs/file.c | 8 ++------
fs/udf/file.c | 11 ++---------
fs/xfs/xfs_file.c | 8 +-------
include/linux/fs.h | 8 +-------
mm/filemap.c | 30 ++++++++++++++++++++----------
11 files changed, 40 insertions(+), 73 deletions(-)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index cc9d411..c529b1c 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
*/
ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
- struct file *file = iocb->ki_filp;
struct blk_plug plug;
ssize_t ret;
blk_start_plug(&plug);
ret = __generic_file_write_iter(iocb, from);
- if (ret > 0) {
- ssize_t err;
- err = generic_write_sync(file, iocb->ki_pos - ret, ret);
- if (err < 0)
- ret = err;
- }
+ ret = generic_write_sync(iocb, ret);
blk_finish_plug(&plug);
return ret;
}
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index a18ceab..4f4a6f7 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
*/
BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
BTRFS_I(inode)->last_sub_trans = root->log_transid;
- if (num_written > 0) {
- err = generic_write_sync(file, pos, num_written);
- if (err < 0)
- num_written = err;
- }
+
+ num_written = generic_write_sync(iocb, num_written);
if (sync)
atomic_dec(&BTRFS_I(inode)->sync_writers);
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c485afa..32359de 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
rc = __generic_file_write_iter(iocb, from);
mutex_unlock(&inode->i_mutex);
- if (rc > 0) {
- ssize_t err;
-
- err = generic_write_sync(file, iocb->ki_pos - rc, rc);
- if (err < 0)
- rc = err;
- }
+ rc = generic_write_sync(iocb, rc);
} else {
mutex_unlock(&inode->i_mutex);
}
diff --git a/fs/direct-io.c b/fs/direct-io.c
index e181b6b..b72ac83 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
inode_dio_done(dio->inode);
if (is_async) {
if (dio->rw & WRITE) {
- int err;
-
- err = generic_write_sync(dio->iocb->ki_filp, offset,
- transferred);
- if (err < 0 && ret > 0)
- ret = err;
+ dio->iocb->ki_pos = offset + transferred;
+ ret = generic_write_sync(dio->iocb, ret);
}
aio_complete(dio->iocb, ret, 0);
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index aca7b24..79b000c 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
ret = __generic_file_write_iter(iocb, from);
mutex_unlock(&inode->i_mutex);
- if (ret > 0) {
- ssize_t err;
-
- err = generic_write_sync(file, iocb->ki_pos - ret, ret);
- if (err < 0)
- ret = err;
- }
+ ret = generic_write_sync(iocb, ret);
if (o_direct)
blk_finish_plug(&plug);
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index 80dd44d..3fafeca 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -895,8 +895,13 @@ retry:
gfs2_quota_unlock(ip);
}
- if (error == 0)
- error = generic_write_sync(file, pos, count);
+ if (error)
+ goto out_unlock;
+
+ if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
+ error = vfs_fsync_range(file, pos, pos + count - 1,
+ (file->f_flags & __O_SYNC) ? 0 : 1);
+ }
goto out_unlock;
out_trans_fail:
diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 643faa4..4f3d664 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
mutex_lock(&inode->i_mutex);
ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
mutex_unlock(&inode->i_mutex);
- if (ret > 0) {
- int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
- if (err < 0)
- ret = err;
- }
- return ret;
+
+ return generic_write_sync(iocb, ret);
}
/**
diff --git a/fs/udf/file.c b/fs/udf/file.c
index bb15771..1cdabd0 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
retval = __generic_file_write_iter(iocb, from);
mutex_unlock(&inode->i_mutex);
- if (retval > 0) {
- ssize_t err;
-
+ if (retval > 0)
mark_inode_dirty(inode);
- err = generic_write_sync(file, iocb->ki_pos - retval, retval);
- if (err < 0)
- retval = err;
- }
-
- return retval;
+ return generic_write_sync(iocb, retval);
}
long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 0655915..a8cab66 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -792,14 +792,8 @@ xfs_file_write_iter(
ret = xfs_file_buffered_aio_write(iocb, from);
if (ret > 0) {
- ssize_t err;
-
XFS_STATS_ADD(xs_write_bytes, ret);
-
- /* Handle various SYNC-type writes */
- err = generic_write_sync(file, iocb->ki_pos - ret, ret);
- if (err < 0)
- ret = err;
+ ret = generic_write_sync(iocb, ret);
}
return ret;
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index eaebd99..7d0e116 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
int datasync);
extern int vfs_fsync(struct file *file, int datasync);
-static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
-{
- if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
- return 0;
- return vfs_fsync_range(file, pos, pos + count - 1,
- (file->f_flags & __O_SYNC) ? 0 : 1);
-}
+extern int generic_write_sync(struct kiocb *iocb, loff_t count);
extern void emergency_sync(void);
extern void emergency_remount(void);
#ifdef CONFIG_BLOCK
diff --git a/mm/filemap.c b/mm/filemap.c
index 09d3af3..6107058 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2664,6 +2664,24 @@ out:
}
EXPORT_SYMBOL(__generic_file_write_iter);
+int generic_write_sync(struct kiocb *iocb, loff_t count)
+{
+ struct file *file = iocb->ki_filp;
+
+ if (count > 0 &&
+ ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
+ bool fdatasync = !(file->f_flags & __O_SYNC);
+ ssize_t ret = 0;
+
+ ret = vfs_fsync_range(file, iocb->ki_pos - count,
+ iocb->ki_pos - 1, fdatasync);
+ if (ret < 0)
+ return ret;
+ }
+ return count;
+}
+EXPORT_SYMBOL(generic_write_sync);
+
/**
* generic_file_write_iter - write data to a file
* @iocb: IO state structure
@@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
*/
ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
- struct file *file = iocb->ki_filp;
- struct inode *inode = file->f_mapping->host;
+ struct inode *inode = iocb->ki_filp->f_mapping->host;
ssize_t ret;
mutex_lock(&inode->i_mutex);
ret = __generic_file_write_iter(iocb, from);
mutex_unlock(&inode->i_mutex);
- if (ret > 0) {
- ssize_t err;
-
- err = generic_write_sync(file, iocb->ki_pos - ret, ret);
- if (err < 0)
- ret = err;
- }
- return ret;
+ return generic_write_sync(iocb, ret);
}
EXPORT_SYMBOL(generic_file_write_iter);
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Cluster-devel] [PATCH v5 6/7] fs: pass iocb to generic_write_sync
2014-11-05 21:14 ` [PATCH v5 6/7] fs: pass iocb to generic_write_sync Milosz Tanski
@ 2014-11-06 10:18 ` Steven Whitehouse
2014-11-06 10:52 ` [Linux-NTFS-Dev] " Anton Altaparmakov
2014-11-06 12:04 ` Jan Kara
2 siblings, 0 replies; 7+ messages in thread
From: Steven Whitehouse @ 2014-11-06 10:18 UTC (permalink / raw)
To: Milosz Tanski, linux-kernel
Cc: linux-arch, linux-aio, Volker Lendecke, Theodore Ts'o,
linux-xfs, linux-cifs, linux-ntfs-dev, linux-api, Tejun Heo,
Jeff Moyer, cluster-devel, Mel Gorman, linux-fsdevel,
Michael Kerrisk, linux-ext4, Christoph Hellwig, linux-btrfs,
Al Viro, Andrew Price
Hi,
On 05/11/14 21:14, Milosz Tanski wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Clean up the generic_write_sync by just passing an iocb and a bytes
> written / negative errno argument. In addition to simplifying the
> callers this also prepares for passing a per-operation O_DSYNC
> flag. Two callers didn't quite fit that scheme:
>
> - dio_complete didn't both to update ki_pos as we don't need it
> on a iocb that is about to be freed, so we had to add it. Additionally
> it also synced out written data in the error case, which has been
> changed to operate like the other callers.
> - gfs2 also used generic_write_sync to implement a crude version
> of fallocate. It has been switched to use an open coded variant
> instead.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
GFS2 bits:
Acked-by: Steven Whitehouse <swhiteho@redhat.com>
I know that Andy Price has some work in this area too, so in due course
we'll have to be careful not to create a merge conflict here. Copying in
Andy so he can see the changes,
Steve.
> ---
> fs/block_dev.c | 8 +-------
> fs/btrfs/file.c | 7 ++-----
> fs/cifs/file.c | 8 +-------
> fs/direct-io.c | 8 ++------
> fs/ext4/file.c | 8 +-------
> fs/gfs2/file.c | 9 +++++++--
> fs/ntfs/file.c | 8 ++------
> fs/udf/file.c | 11 ++---------
> fs/xfs/xfs_file.c | 8 +-------
> include/linux/fs.h | 8 +-------
> mm/filemap.c | 30 ++++++++++++++++++++----------
> 11 files changed, 40 insertions(+), 73 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index cc9d411..c529b1c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> */
> ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> struct blk_plug plug;
> ssize_t ret;
>
> blk_start_plug(&plug);
> ret = __generic_file_write_iter(iocb, from);
> - if (ret > 0) {
> - ssize_t err;
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> blk_finish_plug(&plug);
> return ret;
> }
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index a18ceab..4f4a6f7 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
> */
> BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
> BTRFS_I(inode)->last_sub_trans = root->log_transid;
> - if (num_written > 0) {
> - err = generic_write_sync(file, pos, num_written);
> - if (err < 0)
> - num_written = err;
> - }
> +
> + num_written = generic_write_sync(iocb, num_written);
>
> if (sync)
> atomic_dec(&BTRFS_I(inode)->sync_writers);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index c485afa..32359de 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> rc = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (rc > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - rc, rc);
> - if (err < 0)
> - rc = err;
> - }
> + rc = generic_write_sync(iocb, rc);
> } else {
> mutex_unlock(&inode->i_mutex);
> }
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index e181b6b..b72ac83 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
> inode_dio_done(dio->inode);
> if (is_async) {
> if (dio->rw & WRITE) {
> - int err;
> -
> - err = generic_write_sync(dio->iocb->ki_filp, offset,
> - transferred);
> - if (err < 0 && ret > 0)
> - ret = err;
> + dio->iocb->ki_pos = offset + transferred;
> + ret = generic_write_sync(dio->iocb, ret);
> }
>
> aio_complete(dio->iocb, ret, 0);
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index aca7b24..79b000c 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> if (o_direct)
> blk_finish_plug(&plug);
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 80dd44d..3fafeca 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -895,8 +895,13 @@ retry:
> gfs2_quota_unlock(ip);
> }
>
> - if (error == 0)
> - error = generic_write_sync(file, pos, count);
> + if (error)
> + goto out_unlock;
> +
> + if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
> + error = vfs_fsync_range(file, pos, pos + count - 1,
> + (file->f_flags & __O_SYNC) ? 0 : 1);
> + }
> goto out_unlock;
>
> out_trans_fail:
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 643faa4..4f3d664 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
> mutex_lock(&inode->i_mutex);
> ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
> mutex_unlock(&inode->i_mutex);
> - if (ret > 0) {
> - int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> +
> + return generic_write_sync(iocb, ret);
> }
>
> /**
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index bb15771..1cdabd0 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> retval = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (retval > 0) {
> - ssize_t err;
> -
> + if (retval > 0)
> mark_inode_dirty(inode);
> - err = generic_write_sync(file, iocb->ki_pos - retval, retval);
> - if (err < 0)
> - retval = err;
> - }
> -
> - return retval;
> + return generic_write_sync(iocb, retval);
> }
>
> long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 0655915..a8cab66 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -792,14 +792,8 @@ xfs_file_write_iter(
> ret = xfs_file_buffered_aio_write(iocb, from);
>
> if (ret > 0) {
> - ssize_t err;
> -
> XFS_STATS_ADD(xs_write_bytes, ret);
> -
> - /* Handle various SYNC-type writes */
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> + ret = generic_write_sync(iocb, ret);
> }
> return ret;
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index eaebd99..7d0e116 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
> extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
> int datasync);
> extern int vfs_fsync(struct file *file, int datasync);
> -static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
> -{
> - if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
> - return 0;
> - return vfs_fsync_range(file, pos, pos + count - 1,
> - (file->f_flags & __O_SYNC) ? 0 : 1);
> -}
> +extern int generic_write_sync(struct kiocb *iocb, loff_t count);
> extern void emergency_sync(void);
> extern void emergency_remount(void);
> #ifdef CONFIG_BLOCK
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 09d3af3..6107058 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2664,6 +2664,24 @@ out:
> }
> EXPORT_SYMBOL(__generic_file_write_iter);
>
> +int generic_write_sync(struct kiocb *iocb, loff_t count)
> +{
> + struct file *file = iocb->ki_filp;
> +
> + if (count > 0 &&
> + ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
> + bool fdatasync = !(file->f_flags & __O_SYNC);
> + ssize_t ret = 0;
> +
> + ret = vfs_fsync_range(file, iocb->ki_pos - count,
> + iocb->ki_pos - 1, fdatasync);
> + if (ret < 0)
> + return ret;
> + }
> + return count;
> +}
> +EXPORT_SYMBOL(generic_write_sync);
> +
> /**
> * generic_file_write_iter - write data to a file
> * @iocb: IO state structure
> @@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
> */
> ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> - struct inode *inode = file->f_mapping->host;
> + struct inode *inode = iocb->ki_filp->f_mapping->host;
> ssize_t ret;
>
> mutex_lock(&inode->i_mutex);
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> + return generic_write_sync(iocb, ret);
> }
> EXPORT_SYMBOL(generic_file_write_iter);
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-NTFS-Dev] [PATCH v5 6/7] fs: pass iocb to generic_write_sync
2014-11-05 21:14 ` [PATCH v5 6/7] fs: pass iocb to generic_write_sync Milosz Tanski
2014-11-06 10:18 ` [Cluster-devel] " Steven Whitehouse
@ 2014-11-06 10:52 ` Anton Altaparmakov
2014-11-06 16:14 ` Milosz Tanski
2014-11-06 12:04 ` Jan Kara
2 siblings, 1 reply; 7+ messages in thread
From: Anton Altaparmakov @ 2014-11-06 10:52 UTC (permalink / raw)
To: Milosz Tanski
Cc: Linux Kernel Mailing List, linux-arch, linux-aio, Volker Lendecke,
Theodore Ts'o, linux-xfs, linux-cifs, linux-ntfs-dev,
linux-api, Christoph Hellwig, Tejun Heo, Jeff Moyer,
cluster-devel, Mel Gorman, linux-fsdevel, Michael Kerrisk,
linux-ext4, Christoph Hellwig, linux-btrfs, Al Viro
Hi,
> On 5 Nov 2014, at 23:14, Milosz Tanski <milosz@adfin.com> wrote:
>
> From: Christoph Hellwig <hch@lst.de>
>
> Clean up the generic_write_sync by just passing an iocb and a bytes
> written / negative errno argument. In addition to simplifying the
> callers this also prepares for passing a per-operation O_DSYNC
> flag. Two callers didn't quite fit that scheme:
>
> - dio_complete didn't both to update ki_pos as we don't need it
> on a iocb that is about to be freed, so we had to add it. Additionally
> it also synced out written data in the error case, which has been
> changed to operate like the other callers.
> - gfs2 also used generic_write_sync to implement a crude version
> of fallocate. It has been switched to use an open coded variant
> instead.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/block_dev.c | 8 +-------
> fs/btrfs/file.c | 7 ++-----
> fs/cifs/file.c | 8 +-------
> fs/direct-io.c | 8 ++------
> fs/ext4/file.c | 8 +-------
> fs/gfs2/file.c | 9 +++++++--
> fs/ntfs/file.c | 8 ++------
> fs/udf/file.c | 11 ++---------
> fs/xfs/xfs_file.c | 8 +-------
> include/linux/fs.h | 8 +-------
> mm/filemap.c | 30 ++++++++++++++++++++----------
> 11 files changed, 40 insertions(+), 73 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index cc9d411..c529b1c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> */
> ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> struct blk_plug plug;
> ssize_t ret;
>
> blk_start_plug(&plug);
> ret = __generic_file_write_iter(iocb, from);
> - if (ret > 0) {
> - ssize_t err;
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> blk_finish_plug(&plug);
> return ret;
> }
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index a18ceab..4f4a6f7 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
> */
> BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
> BTRFS_I(inode)->last_sub_trans = root->log_transid;
> - if (num_written > 0) {
> - err = generic_write_sync(file, pos, num_written);
> - if (err < 0)
> - num_written = err;
> - }
> +
> + num_written = generic_write_sync(iocb, num_written);
>
> if (sync)
> atomic_dec(&BTRFS_I(inode)->sync_writers);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index c485afa..32359de 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> rc = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (rc > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - rc, rc);
> - if (err < 0)
> - rc = err;
> - }
> + rc = generic_write_sync(iocb, rc);
> } else {
> mutex_unlock(&inode->i_mutex);
> }
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index e181b6b..b72ac83 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
> inode_dio_done(dio->inode);
> if (is_async) {
> if (dio->rw & WRITE) {
> - int err;
> -
> - err = generic_write_sync(dio->iocb->ki_filp, offset,
> - transferred);
> - if (err < 0 && ret > 0)
> - ret = err;
> + dio->iocb->ki_pos = offset + transferred;
> + ret = generic_write_sync(dio->iocb, ret);
> }
>
> aio_complete(dio->iocb, ret, 0);
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index aca7b24..79b000c 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> if (o_direct)
> blk_finish_plug(&plug);
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 80dd44d..3fafeca 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -895,8 +895,13 @@ retry:
> gfs2_quota_unlock(ip);
> }
>
> - if (error == 0)
> - error = generic_write_sync(file, pos, count);
> + if (error)
> + goto out_unlock;
> +
> + if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
> + error = vfs_fsync_range(file, pos, pos + count - 1,
> + (file->f_flags & __O_SYNC) ? 0 : 1);
> + }
> goto out_unlock;
>
> out_trans_fail:
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 643faa4..4f3d664 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
> mutex_lock(&inode->i_mutex);
> ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
> mutex_unlock(&inode->i_mutex);
> - if (ret > 0) {
> - int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> +
> + return generic_write_sync(iocb, ret);
> }
>
> /**
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index bb15771..1cdabd0 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> retval = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (retval > 0) {
> - ssize_t err;
> -
> + if (retval > 0)
> mark_inode_dirty(inode);
> - err = generic_write_sync(file, iocb->ki_pos - retval, retval);
> - if (err < 0)
> - retval = err;
> - }
> -
> - return retval;
> + return generic_write_sync(iocb, retval);
> }
>
> long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 0655915..a8cab66 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -792,14 +792,8 @@ xfs_file_write_iter(
> ret = xfs_file_buffered_aio_write(iocb, from);
>
> if (ret > 0) {
> - ssize_t err;
> -
> XFS_STATS_ADD(xs_write_bytes, ret);
> -
> - /* Handle various SYNC-type writes */
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> + ret = generic_write_sync(iocb, ret);
> }
> return ret;
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index eaebd99..7d0e116 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
> extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
> int datasync);
> extern int vfs_fsync(struct file *file, int datasync);
> -static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
> -{
> - if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
> - return 0;
> - return vfs_fsync_range(file, pos, pos + count - 1,
> - (file->f_flags & __O_SYNC) ? 0 : 1);
> -}
> +extern int generic_write_sync(struct kiocb *iocb, loff_t count);
> extern void emergency_sync(void);
> extern void emergency_remount(void);
> #ifdef CONFIG_BLOCK
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 09d3af3..6107058 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2664,6 +2664,24 @@ out:
> }
> EXPORT_SYMBOL(__generic_file_write_iter);
>
> +int generic_write_sync(struct kiocb *iocb, loff_t count)
> +{
> + struct file *file = iocb->ki_filp;
> +
> + if (count > 0 &&
> + ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
> + bool fdatasync = !(file->f_flags & __O_SYNC);
> + ssize_t ret = 0;
That "= 0" is pointless. "ret" is overwritten unconditionally on the following line...
Other than that the NTFS bits are:
Acked-by: Anton Altaparmakov <anton@tuxera.com>
Best regards,
Anton
> +
> + ret = vfs_fsync_range(file, iocb->ki_pos - count,
> + iocb->ki_pos - 1, fdatasync);
> + if (ret < 0)
> + return ret;
> + }
> + return count;
> +}
> +EXPORT_SYMBOL(generic_write_sync);
> +
> /**
> * generic_file_write_iter - write data to a file
> * @iocb: IO state structure
> @@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
> */
> ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> - struct inode *inode = file->f_mapping->host;
> + struct inode *inode = iocb->ki_filp->f_mapping->host;
> ssize_t ret;
>
> mutex_lock(&inode->i_mutex);
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> + return generic_write_sync(iocb, ret);
> }
> EXPORT_SYMBOL(generic_file_write_iter);
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
University of Cambridge Information Services, Roger Needham Building
7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 6/7] fs: pass iocb to generic_write_sync
2014-11-05 21:14 ` [PATCH v5 6/7] fs: pass iocb to generic_write_sync Milosz Tanski
2014-11-06 10:18 ` [Cluster-devel] " Steven Whitehouse
2014-11-06 10:52 ` [Linux-NTFS-Dev] " Anton Altaparmakov
@ 2014-11-06 12:04 ` Jan Kara
2 siblings, 0 replies; 7+ messages in thread
From: Jan Kara @ 2014-11-06 12:04 UTC (permalink / raw)
To: Milosz Tanski
Cc: linux-kernel, Christoph Hellwig, Christoph Hellwig, linux-fsdevel,
linux-aio, Mel Gorman, Volker Lendecke, Tejun Heo, Jeff Moyer,
Theodore Ts'o, Al Viro, linux-api, Michael Kerrisk,
linux-arch, linux-btrfs, linux-cifs, linux-ext4, linux-ntfs-dev,
linux-xfs, cluster-devel
On Wed 05-11-14 16:14:52, Milosz Tanski wrote:
> From: Christoph Hellwig <hch@lst.de>
>
> Clean up the generic_write_sync by just passing an iocb and a bytes
> written / negative errno argument. In addition to simplifying the
> callers this also prepares for passing a per-operation O_DSYNC
> flag. Two callers didn't quite fit that scheme:
>
> - dio_complete didn't both to update ki_pos as we don't need it
> on a iocb that is about to be freed, so we had to add it. Additionally
> it also synced out written data in the error case, which has been
> changed to operate like the other callers.
> - gfs2 also used generic_write_sync to implement a crude version
> of fallocate. It has been switched to use an open coded variant
> instead.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good. You can add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/block_dev.c | 8 +-------
> fs/btrfs/file.c | 7 ++-----
> fs/cifs/file.c | 8 +-------
> fs/direct-io.c | 8 ++------
> fs/ext4/file.c | 8 +-------
> fs/gfs2/file.c | 9 +++++++--
> fs/ntfs/file.c | 8 ++------
> fs/udf/file.c | 11 ++---------
> fs/xfs/xfs_file.c | 8 +-------
> include/linux/fs.h | 8 +-------
> mm/filemap.c | 30 ++++++++++++++++++++----------
> 11 files changed, 40 insertions(+), 73 deletions(-)
>
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index cc9d411..c529b1c 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
> */
> ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> struct blk_plug plug;
> ssize_t ret;
>
> blk_start_plug(&plug);
> ret = __generic_file_write_iter(iocb, from);
> - if (ret > 0) {
> - ssize_t err;
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> blk_finish_plug(&plug);
> return ret;
> }
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index a18ceab..4f4a6f7 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
> */
> BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
> BTRFS_I(inode)->last_sub_trans = root->log_transid;
> - if (num_written > 0) {
> - err = generic_write_sync(file, pos, num_written);
> - if (err < 0)
> - num_written = err;
> - }
> +
> + num_written = generic_write_sync(iocb, num_written);
>
> if (sync)
> atomic_dec(&BTRFS_I(inode)->sync_writers);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index c485afa..32359de 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
> rc = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (rc > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - rc, rc);
> - if (err < 0)
> - rc = err;
> - }
> + rc = generic_write_sync(iocb, rc);
> } else {
> mutex_unlock(&inode->i_mutex);
> }
> diff --git a/fs/direct-io.c b/fs/direct-io.c
> index e181b6b..b72ac83 100644
> --- a/fs/direct-io.c
> +++ b/fs/direct-io.c
> @@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
> inode_dio_done(dio->inode);
> if (is_async) {
> if (dio->rw & WRITE) {
> - int err;
> -
> - err = generic_write_sync(dio->iocb->ki_filp, offset,
> - transferred);
> - if (err < 0 && ret > 0)
> - ret = err;
> + dio->iocb->ki_pos = offset + transferred;
> + ret = generic_write_sync(dio->iocb, ret);
> }
>
> aio_complete(dio->iocb, ret, 0);
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index aca7b24..79b000c 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> + ret = generic_write_sync(iocb, ret);
> if (o_direct)
> blk_finish_plug(&plug);
>
> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
> index 80dd44d..3fafeca 100644
> --- a/fs/gfs2/file.c
> +++ b/fs/gfs2/file.c
> @@ -895,8 +895,13 @@ retry:
> gfs2_quota_unlock(ip);
> }
>
> - if (error == 0)
> - error = generic_write_sync(file, pos, count);
> + if (error)
> + goto out_unlock;
> +
> + if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
> + error = vfs_fsync_range(file, pos, pos + count - 1,
> + (file->f_flags & __O_SYNC) ? 0 : 1);
> + }
> goto out_unlock;
>
> out_trans_fail:
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 643faa4..4f3d664 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
> mutex_lock(&inode->i_mutex);
> ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
> mutex_unlock(&inode->i_mutex);
> - if (ret > 0) {
> - int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> +
> + return generic_write_sync(iocb, ret);
> }
>
> /**
> diff --git a/fs/udf/file.c b/fs/udf/file.c
> index bb15771..1cdabd0 100644
> --- a/fs/udf/file.c
> +++ b/fs/udf/file.c
> @@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> retval = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (retval > 0) {
> - ssize_t err;
> -
> + if (retval > 0)
> mark_inode_dirty(inode);
> - err = generic_write_sync(file, iocb->ki_pos - retval, retval);
> - if (err < 0)
> - retval = err;
> - }
> -
> - return retval;
> + return generic_write_sync(iocb, retval);
> }
>
> long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 0655915..a8cab66 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -792,14 +792,8 @@ xfs_file_write_iter(
> ret = xfs_file_buffered_aio_write(iocb, from);
>
> if (ret > 0) {
> - ssize_t err;
> -
> XFS_STATS_ADD(xs_write_bytes, ret);
> -
> - /* Handle various SYNC-type writes */
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> + ret = generic_write_sync(iocb, ret);
> }
> return ret;
> }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index eaebd99..7d0e116 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
> extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
> int datasync);
> extern int vfs_fsync(struct file *file, int datasync);
> -static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
> -{
> - if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
> - return 0;
> - return vfs_fsync_range(file, pos, pos + count - 1,
> - (file->f_flags & __O_SYNC) ? 0 : 1);
> -}
> +extern int generic_write_sync(struct kiocb *iocb, loff_t count);
> extern void emergency_sync(void);
> extern void emergency_remount(void);
> #ifdef CONFIG_BLOCK
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 09d3af3..6107058 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2664,6 +2664,24 @@ out:
> }
> EXPORT_SYMBOL(__generic_file_write_iter);
>
> +int generic_write_sync(struct kiocb *iocb, loff_t count)
> +{
> + struct file *file = iocb->ki_filp;
> +
> + if (count > 0 &&
> + ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
> + bool fdatasync = !(file->f_flags & __O_SYNC);
> + ssize_t ret = 0;
> +
> + ret = vfs_fsync_range(file, iocb->ki_pos - count,
> + iocb->ki_pos - 1, fdatasync);
> + if (ret < 0)
> + return ret;
> + }
> + return count;
> +}
> +EXPORT_SYMBOL(generic_write_sync);
> +
> /**
> * generic_file_write_iter - write data to a file
> * @iocb: IO state structure
> @@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
> */
> ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> {
> - struct file *file = iocb->ki_filp;
> - struct inode *inode = file->f_mapping->host;
> + struct inode *inode = iocb->ki_filp->f_mapping->host;
> ssize_t ret;
>
> mutex_lock(&inode->i_mutex);
> ret = __generic_file_write_iter(iocb, from);
> mutex_unlock(&inode->i_mutex);
>
> - if (ret > 0) {
> - ssize_t err;
> -
> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
> - if (err < 0)
> - ret = err;
> - }
> - return ret;
> + return generic_write_sync(iocb, ret);
> }
> EXPORT_SYMBOL(generic_file_write_iter);
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Linux-NTFS-Dev] [PATCH v5 6/7] fs: pass iocb to generic_write_sync
2014-11-06 10:52 ` [Linux-NTFS-Dev] " Anton Altaparmakov
@ 2014-11-06 16:14 ` Milosz Tanski
0 siblings, 0 replies; 7+ messages in thread
From: Milosz Tanski @ 2014-11-06 16:14 UTC (permalink / raw)
To: Anton Altaparmakov
Cc: Linux Kernel Mailing List, linux-arch, linux-aio@kvack.org,
Volker Lendecke, Theodore Ts'o, linux-xfs, linux-cifs,
linux-ntfs-dev, Linux API, Christoph Hellwig, Tejun Heo,
Jeff Moyer, cluster-devel, Mel Gorman,
linux-fsdevel@vger.kernel.org, Michael Kerrisk, linux-ext4,
Christoph Hellwig, linux-btrfs, Al Viro
On Thu, Nov 6, 2014 at 5:52 AM, Anton Altaparmakov <aia21@cam.ac.uk> wrote:
> Hi,
>
>> On 5 Nov 2014, at 23:14, Milosz Tanski <milosz@adfin.com> wrote:
>>
>> From: Christoph Hellwig <hch@lst.de>
>>
>> Clean up the generic_write_sync by just passing an iocb and a bytes
>> written / negative errno argument. In addition to simplifying the
>> callers this also prepares for passing a per-operation O_DSYNC
>> flag. Two callers didn't quite fit that scheme:
>>
>> - dio_complete didn't both to update ki_pos as we don't need it
>> on a iocb that is about to be freed, so we had to add it. Additionally
>> it also synced out written data in the error case, which has been
>> changed to operate like the other callers.
>> - gfs2 also used generic_write_sync to implement a crude version
>> of fallocate. It has been switched to use an open coded variant
>> instead.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>> fs/block_dev.c | 8 +-------
>> fs/btrfs/file.c | 7 ++-----
>> fs/cifs/file.c | 8 +-------
>> fs/direct-io.c | 8 ++------
>> fs/ext4/file.c | 8 +-------
>> fs/gfs2/file.c | 9 +++++++--
>> fs/ntfs/file.c | 8 ++------
>> fs/udf/file.c | 11 ++---------
>> fs/xfs/xfs_file.c | 8 +-------
>> include/linux/fs.h | 8 +-------
>> mm/filemap.c | 30 ++++++++++++++++++++----------
>> 11 files changed, 40 insertions(+), 73 deletions(-)
>>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index cc9d411..c529b1c 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -1568,18 +1568,12 @@ static long block_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>> */
>> ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> {
>> - struct file *file = iocb->ki_filp;
>> struct blk_plug plug;
>> ssize_t ret;
>>
>> blk_start_plug(&plug);
>> ret = __generic_file_write_iter(iocb, from);
>> - if (ret > 0) {
>> - ssize_t err;
>> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> - if (err < 0)
>> - ret = err;
>> - }
>> + ret = generic_write_sync(iocb, ret);
>> blk_finish_plug(&plug);
>> return ret;
>> }
>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
>> index a18ceab..4f4a6f7 100644
>> --- a/fs/btrfs/file.c
>> +++ b/fs/btrfs/file.c
>> @@ -1820,11 +1820,8 @@ static ssize_t btrfs_file_write_iter(struct kiocb *iocb,
>> */
>> BTRFS_I(inode)->last_trans = root->fs_info->generation + 1;
>> BTRFS_I(inode)->last_sub_trans = root->log_transid;
>> - if (num_written > 0) {
>> - err = generic_write_sync(file, pos, num_written);
>> - if (err < 0)
>> - num_written = err;
>> - }
>> +
>> + num_written = generic_write_sync(iocb, num_written);
>>
>> if (sync)
>> atomic_dec(&BTRFS_I(inode)->sync_writers);
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index c485afa..32359de 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -2706,13 +2706,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from)
>> rc = __generic_file_write_iter(iocb, from);
>> mutex_unlock(&inode->i_mutex);
>>
>> - if (rc > 0) {
>> - ssize_t err;
>> -
>> - err = generic_write_sync(file, iocb->ki_pos - rc, rc);
>> - if (err < 0)
>> - rc = err;
>> - }
>> + rc = generic_write_sync(iocb, rc);
>> } else {
>> mutex_unlock(&inode->i_mutex);
>> }
>> diff --git a/fs/direct-io.c b/fs/direct-io.c
>> index e181b6b..b72ac83 100644
>> --- a/fs/direct-io.c
>> +++ b/fs/direct-io.c
>> @@ -257,12 +257,8 @@ static ssize_t dio_complete(struct dio *dio, loff_t offset, ssize_t ret,
>> inode_dio_done(dio->inode);
>> if (is_async) {
>> if (dio->rw & WRITE) {
>> - int err;
>> -
>> - err = generic_write_sync(dio->iocb->ki_filp, offset,
>> - transferred);
>> - if (err < 0 && ret > 0)
>> - ret = err;
>> + dio->iocb->ki_pos = offset + transferred;
>> + ret = generic_write_sync(dio->iocb, ret);
>> }
>>
>> aio_complete(dio->iocb, ret, 0);
>> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
>> index aca7b24..79b000c 100644
>> --- a/fs/ext4/file.c
>> +++ b/fs/ext4/file.c
>> @@ -175,13 +175,7 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> ret = __generic_file_write_iter(iocb, from);
>> mutex_unlock(&inode->i_mutex);
>>
>> - if (ret > 0) {
>> - ssize_t err;
>> -
>> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> - if (err < 0)
>> - ret = err;
>> - }
>> + ret = generic_write_sync(iocb, ret);
>> if (o_direct)
>> blk_finish_plug(&plug);
>>
>> diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
>> index 80dd44d..3fafeca 100644
>> --- a/fs/gfs2/file.c
>> +++ b/fs/gfs2/file.c
>> @@ -895,8 +895,13 @@ retry:
>> gfs2_quota_unlock(ip);
>> }
>>
>> - if (error == 0)
>> - error = generic_write_sync(file, pos, count);
>> + if (error)
>> + goto out_unlock;
>> +
>> + if ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host)) {
>> + error = vfs_fsync_range(file, pos, pos + count - 1,
>> + (file->f_flags & __O_SYNC) ? 0 : 1);
>> + }
>> goto out_unlock;
>>
>> out_trans_fail:
>> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> index 643faa4..4f3d664 100644
>> --- a/fs/ntfs/file.c
>> +++ b/fs/ntfs/file.c
>> @@ -2127,12 +2127,8 @@ static ssize_t ntfs_file_aio_write(struct kiocb *iocb, const struct iovec *iov,
>> mutex_lock(&inode->i_mutex);
>> ret = ntfs_file_aio_write_nolock(iocb, iov, nr_segs, &iocb->ki_pos);
>> mutex_unlock(&inode->i_mutex);
>> - if (ret > 0) {
>> - int err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> - if (err < 0)
>> - ret = err;
>> - }
>> - return ret;
>> +
>> + return generic_write_sync(iocb, ret);
>> }
>>
>> /**
>> diff --git a/fs/udf/file.c b/fs/udf/file.c
>> index bb15771..1cdabd0 100644
>> --- a/fs/udf/file.c
>> +++ b/fs/udf/file.c
>> @@ -155,16 +155,9 @@ static ssize_t udf_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> retval = __generic_file_write_iter(iocb, from);
>> mutex_unlock(&inode->i_mutex);
>>
>> - if (retval > 0) {
>> - ssize_t err;
>> -
>> + if (retval > 0)
>> mark_inode_dirty(inode);
>> - err = generic_write_sync(file, iocb->ki_pos - retval, retval);
>> - if (err < 0)
>> - retval = err;
>> - }
>> -
>> - return retval;
>> + return generic_write_sync(iocb, retval);
>> }
>>
>> long udf_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
>> index 0655915..a8cab66 100644
>> --- a/fs/xfs/xfs_file.c
>> +++ b/fs/xfs/xfs_file.c
>> @@ -792,14 +792,8 @@ xfs_file_write_iter(
>> ret = xfs_file_buffered_aio_write(iocb, from);
>>
>> if (ret > 0) {
>> - ssize_t err;
>> -
>> XFS_STATS_ADD(xs_write_bytes, ret);
>> -
>> - /* Handle various SYNC-type writes */
>> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> - if (err < 0)
>> - ret = err;
>> + ret = generic_write_sync(iocb, ret);
>> }
>> return ret;
>> }
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index eaebd99..7d0e116 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2242,13 +2242,7 @@ extern int filemap_fdatawrite_range(struct address_space *mapping,
>> extern int vfs_fsync_range(struct file *file, loff_t start, loff_t end,
>> int datasync);
>> extern int vfs_fsync(struct file *file, int datasync);
>> -static inline int generic_write_sync(struct file *file, loff_t pos, loff_t count)
>> -{
>> - if (!(file->f_flags & O_DSYNC) && !IS_SYNC(file->f_mapping->host))
>> - return 0;
>> - return vfs_fsync_range(file, pos, pos + count - 1,
>> - (file->f_flags & __O_SYNC) ? 0 : 1);
>> -}
>> +extern int generic_write_sync(struct kiocb *iocb, loff_t count);
>> extern void emergency_sync(void);
>> extern void emergency_remount(void);
>> #ifdef CONFIG_BLOCK
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index 09d3af3..6107058 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -2664,6 +2664,24 @@ out:
>> }
>> EXPORT_SYMBOL(__generic_file_write_iter);
>>
>> +int generic_write_sync(struct kiocb *iocb, loff_t count)
>> +{
>> + struct file *file = iocb->ki_filp;
>> +
>> + if (count > 0 &&
>> + ((file->f_flags & O_DSYNC) || IS_SYNC(file->f_mapping->host))) {
>> + bool fdatasync = !(file->f_flags & __O_SYNC);
>> + ssize_t ret = 0;
>
> That "= 0" is pointless. "ret" is overwritten unconditionally on the following line...
I have fixed this change; it will be in the next patch series / pull
request. The branch for the pull is at:
https://bitbucket.org/adfin/linux-fs.git read_call_6
>
> Other than that the NTFS bits are:
>
> Acked-by: Anton Altaparmakov <anton@tuxera.com>
>
> Best regards,
>
> Anton
>
>> +
>> + ret = vfs_fsync_range(file, iocb->ki_pos - count,
>> + iocb->ki_pos - 1, fdatasync);
>> + if (ret < 0)
>> + return ret;
>> + }
>> + return count;
>> +}
>> +EXPORT_SYMBOL(generic_write_sync);
>> +
>> /**
>> * generic_file_write_iter - write data to a file
>> * @iocb: IO state structure
>> @@ -2675,22 +2693,14 @@ EXPORT_SYMBOL(__generic_file_write_iter);
>> */
>> ssize_t generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>> {
>> - struct file *file = iocb->ki_filp;
>> - struct inode *inode = file->f_mapping->host;
>> + struct inode *inode = iocb->ki_filp->f_mapping->host;
>> ssize_t ret;
>>
>> mutex_lock(&inode->i_mutex);
>> ret = __generic_file_write_iter(iocb, from);
>> mutex_unlock(&inode->i_mutex);
>>
>> - if (ret > 0) {
>> - ssize_t err;
>> -
>> - err = generic_write_sync(file, iocb->ki_pos - ret, ret);
>> - if (err < 0)
>> - ret = err;
>> - }
>> - return ret;
>> + return generic_write_sync(iocb, ret);
>> }
>> EXPORT_SYMBOL(generic_file_write_iter);
>
> --
> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
> University of Cambridge Information Services, Roger Needham Building
> 7 JJ Thomson Avenue, Cambridge, CB3 0RB, UK
>
--
Milosz Tanski
CTO
16 East 34th Street, 15th floor
New York, NY 10016
p: 646-253-9055
e: milosz@adfin.com
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 4/7] vfs: RWF_NONBLOCK flag for preadv2
2014-11-05 21:14 ` [PATCH v5 4/7] vfs: RWF_NONBLOCK flag for preadv2 Milosz Tanski
@ 2014-11-10 16:07 ` Sage Weil
0 siblings, 0 replies; 7+ messages in thread
From: Sage Weil @ 2014-11-10 16:07 UTC (permalink / raw)
To: Milosz Tanski
Cc: linux-kernel, Christoph Hellwig, linux-fsdevel, linux-aio,
Mel Gorman, Volker Lendecke, Tejun Heo, Jeff Moyer,
Theodore Ts'o, Al Viro, linux-api, Michael Kerrisk,
linux-arch, ceph-devel, linux-cifs, samba-technical, linux-nfs,
linux-xfs, ocfs2-devel, linux-mm
On Wed, 5 Nov 2014, Milosz Tanski wrote:
> generic_file_read_iter() supports a new flag RWF_NONBLOCK which says that we
> only want to read the data if it's already in the page cache.
>
> Additionally, there are a few filesystems that we have to specifically
> bail early if RWF_NONBLOCK because the op would block. Christoph Hellwig
> contributed this code.
>
> Signed-off-by: Milosz Tanski <milosz@adfin.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
Ceph bits
Acked-by: Sage Weil <sage@redhat.com>
> ---
> fs/ceph/file.c | 2 ++
> fs/cifs/file.c | 6 ++++++
> fs/nfs/file.c | 5 ++++-
> fs/ocfs2/file.c | 6 ++++++
> fs/pipe.c | 3 ++-
> fs/read_write.c | 38 +++++++++++++++++++++++++-------------
> fs/xfs/xfs_file.c | 4 ++++
> include/linux/fs.h | 3 +++
> mm/filemap.c | 18 ++++++++++++++++++
> mm/shmem.c | 4 ++++
> 10 files changed, 74 insertions(+), 15 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index d7e0da8..b798b5c 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -822,6 +822,8 @@ again:
> if ((got & (CEPH_CAP_FILE_CACHE|CEPH_CAP_FILE_LAZYIO)) == 0 ||
> (iocb->ki_filp->f_flags & O_DIRECT) ||
> (fi->flags & CEPH_F_SYNC)) {
> + if (iocb->ki_rwflags & O_NONBLOCK)
> + return -EAGAIN;
>
> dout("aio_sync_read %p %llx.%llx %llu~%u got cap refs on %s\n",
> inode, ceph_vinop(inode), iocb->ki_pos, (unsigned)len,
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 3e4d00a..c485afa 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -3005,6 +3005,9 @@ ssize_t cifs_user_readv(struct kiocb *iocb, struct iov_iter *to)
> struct cifs_readdata *rdata, *tmp;
> struct list_head rdata_list;
>
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
> +
> len = iov_iter_count(to);
> if (!len)
> return 0;
> @@ -3123,6 +3126,9 @@ cifs_strict_readv(struct kiocb *iocb, struct iov_iter *to)
> ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0))
> return generic_file_read_iter(iocb, to);
>
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
> +
> /*
> * We need to hold the sem to be sure nobody modifies lock list
> * with a brlock that prevents reading.
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 2ab6f00..aa9046f 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -171,8 +171,11 @@ 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 (iocb->ki_filp->f_flags & O_DIRECT) {
> + if (iocb->ki_rwflags & O_NONBLOCK)
> + return -EAGAIN;
> return nfs_file_direct_read(iocb, to, iocb->ki_pos);
> + }
>
> dprintk("NFS: read(%pD2, %zu@%lu)\n",
> iocb->ki_filp,
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 324dc93..bb66ca4 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2472,6 +2472,12 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
> filp->f_path.dentry->d_name.name,
> to->nr_segs); /* GRRRRR */
>
> + /*
> + * No non-blocking reads for ocfs2 for now. Might be doable with
> + * non-blocking cluster lock helpers.
> + */
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
>
> if (!inode) {
> ret = -EINVAL;
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 21981e5..212bf68 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -302,7 +302,8 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
> */
> if (ret)
> break;
> - if (filp->f_flags & O_NONBLOCK) {
> + if ((filp->f_flags & O_NONBLOCK) ||
> + (iocb->ki_rwflags & RWF_NONBLOCK)) {
> ret = -EAGAIN;
> break;
> }
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 907735c..cba7d4c 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -835,14 +835,19 @@ static ssize_t do_readv_writev(int type, struct file *file,
> file_start_write(file);
> }
>
> - if (iter_fn)
> + if (iter_fn) {
> ret = do_iter_readv_writev(file, type, iov, nr_segs, tot_len,
> pos, iter_fn, flags);
> - else if (fnv)
> - ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
> - pos, fnv);
> - else
> - ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
> + } else {
> + if (type == READ && (flags & RWF_NONBLOCK))
> + return -EAGAIN;
> +
> + if (fnv)
> + ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
> + pos, fnv);
> + else
> + ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
> + }
>
> if (type != READ)
> file_end_write(file);
> @@ -866,8 +871,10 @@ ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
> return -EBADF;
> if (!(file->f_mode & FMODE_CAN_READ))
> return -EINVAL;
> - if (flags & ~0)
> + if (flags & ~RWF_NONBLOCK)
> return -EINVAL;
> + if ((file->f_flags & O_DIRECT) && (flags & RWF_NONBLOCK))
> + return -EAGAIN;
>
> return do_readv_writev(READ, file, vec, vlen, pos, flags);
> }
> @@ -1069,14 +1076,19 @@ static ssize_t compat_do_readv_writev(int type, struct file *file,
> file_start_write(file);
> }
>
> - if (iter_fn)
> + if (iter_fn) {
> ret = do_iter_readv_writev(file, type, iov, nr_segs, tot_len,
> pos, iter_fn, flags);
> - else if (fnv)
> - ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
> - pos, fnv);
> - else
> - ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
> + } else {
> + if (type == READ && (flags & RWF_NONBLOCK))
> + return -EAGAIN;
> +
> + if (fnv)
> + ret = do_sync_readv_writev(file, iov, nr_segs, tot_len,
> + pos, fnv);
> + else
> + ret = do_loop_readv_writev(file, iov, nr_segs, pos, fn);
> + }
>
> if (type != READ)
> file_end_write(file);
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index eb596b4..b1f6334 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -246,6 +246,10 @@ xfs_file_read_iter(
>
> XFS_STATS_INC(xs_read_calls);
>
> + /* XXX: need a non-blocking iolock helper, shouldn't be too hard */
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
> +
> if (unlikely(file->f_flags & O_DIRECT))
> ioflags |= XFS_IO_ISDIRECT;
> if (file->f_mode & FMODE_NOCMTIME)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9ed5711..eaebd99 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1459,6 +1459,9 @@ struct block_device_operations;
> #define HAVE_COMPAT_IOCTL 1
> #define HAVE_UNLOCKED_IOCTL 1
>
> +/* These flags are used for the readv/writev syscalls with flags. */
> +#define RWF_NONBLOCK 0x00000001
> +
> struct iov_iter;
>
> struct file_operations {
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 530c263..09d3af3 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1494,6 +1494,8 @@ static ssize_t do_generic_file_read(struct file *filp, loff_t *ppos,
> find_page:
> page = find_get_page(mapping, index);
> if (!page) {
> + if (flags & RWF_NONBLOCK)
> + goto would_block;
> page_cache_sync_readahead(mapping,
> ra, filp,
> index, last_index - index);
> @@ -1585,6 +1587,11 @@ page_ok:
> continue;
>
> page_not_up_to_date:
> + if (flags & RWF_NONBLOCK) {
> + page_cache_release(page);
> + goto would_block;
> + }
> +
> /* Get exclusive access to the page ... */
> error = lock_page_killable(page);
> if (unlikely(error))
> @@ -1604,6 +1611,12 @@ page_not_up_to_date_locked:
> goto page_ok;
> }
>
> + if (flags & RWF_NONBLOCK) {
> + unlock_page(page);
> + page_cache_release(page);
> + goto would_block;
> + }
> +
> readpage:
> /*
> * A previous I/O error may have been due to temporary
> @@ -1674,6 +1687,8 @@ no_cached_page:
> goto readpage;
> }
>
> +would_block:
> + error = -EAGAIN;
> out:
> ra->prev_pos = prev_index;
> ra->prev_pos <<= PAGE_CACHE_SHIFT;
> @@ -1707,6 +1722,9 @@ generic_file_read_iter(struct kiocb *iocb, struct iov_iter *iter)
> size_t count = iov_iter_count(iter);
> loff_t size;
>
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
> +
> if (!count)
> goto out; /* skip atime */
> size = i_size_read(inode);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index cd6fc75..5c30f04 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1531,6 +1531,10 @@ static ssize_t shmem_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> ssize_t retval = 0;
> loff_t *ppos = &iocb->ki_pos;
>
> + /* XXX: should be easily supportable */
> + if (iocb->ki_rwflags & RWF_NONBLOCK)
> + return -EAGAIN;
> +
> /*
> * Might this read be for a stacking filesystem? Then when reading
> * holes of a sparse file, we actually need to allocate those pages,
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-11-10 16:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1415220890.git.milosz@adfin.com>
2014-11-05 21:14 ` [PATCH v5 4/7] vfs: RWF_NONBLOCK flag for preadv2 Milosz Tanski
2014-11-10 16:07 ` Sage Weil
2014-11-05 21:14 ` [PATCH v5 6/7] fs: pass iocb to generic_write_sync Milosz Tanski
2014-11-06 10:18 ` [Cluster-devel] " Steven Whitehouse
2014-11-06 10:52 ` [Linux-NTFS-Dev] " Anton Altaparmakov
2014-11-06 16:14 ` Milosz Tanski
2014-11-06 12:04 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox