diff mbox series

[v2,10/16] fs: move file_start_write() into vfs_iter_write()

Message ID 20231122122715.2561213-11-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Tidy up file permission hooks | expand

Commit Message

Amir Goldstein Nov. 22, 2023, 12:27 p.m. UTC 
All the callers of vfs_iter_write() call file_start_write() just before
calling vfs_iter_write() except for target_core_file's fd_do_rw().

Move file_start_write() from the callers into vfs_iter_write().
fd_do_rw() calls vfs_iter_write() with a non-regular file, so
file_start_write() is a no-op.

This is needed for fanotify "pre content" events.

Suggested-by: Jan Kara <jack@suse.cz>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 drivers/block/loop.c |  2 --
 fs/coda/file.c       |  2 --
 fs/nfsd/vfs.c        |  2 --
 fs/overlayfs/file.c  |  2 --
 fs/read_write.c      | 13 ++++++++++---
 5 files changed, 10 insertions(+), 11 deletions(-)

Comments

Christoph Hellwig Nov. 23, 2023, 7:50 a.m. UTC | #1 
On Wed, Nov 22, 2023 at 02:27:09PM +0200, Amir Goldstein wrote:
> All the callers of vfs_iter_write() call file_start_write() just before
> calling vfs_iter_write() except for target_core_file's fd_do_rw().

Can you add a patch to first add it to fd_do_rw?  While this crates a
bit of churn, it has two benefits:

 - it gives us an easily backportable fix
 - it makes this patch a transformation that doesn't change behavior.

Please also cc the scsi and target lists.  It probably makes sense to
even expedite it and send it for 6.7-rc inclusion separately.
be a 6.7 candidate
Amir Goldstein Nov. 23, 2023, 8:04 a.m. UTC | #2 
On Thu, Nov 23, 2023 at 9:50 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Nov 22, 2023 at 02:27:09PM +0200, Amir Goldstein wrote:
> > All the callers of vfs_iter_write() call file_start_write() just before
> > calling vfs_iter_write() except for target_core_file's fd_do_rw().
>
> Can you add a patch to first add it to fd_do_rw?  While this crates a
> bit of churn, it has two benefits:
>
>  - it gives us an easily backportable fix
>  - it makes this patch a transformation that doesn't change behavior.
>
> Please also cc the scsi and target lists.  It probably makes sense to
> even expedite it and send it for 6.7-rc inclusion separately.
> be a 6.7 candidate

Good idea. will do!

Thanks for the review!
Amir.
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 9f2d412fc560..8a8cd4fc9238 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -245,9 +245,7 @@  static int lo_write_bvec(struct file *file, struct bio_vec *bvec, loff_t *ppos)
 
 	iov_iter_bvec(&i, ITER_SOURCE, bvec, 1, bvec->bv_len);
 
-	file_start_write(file);
 	bw = vfs_iter_write(file, &i, ppos, 0);
-	file_end_write(file);
 
 	if (likely(bw ==  bvec->bv_len))
 		return 0;
diff --git a/fs/coda/file.c b/fs/coda/file.c
index e62315c37386..148856a582a9 100644
--- a/fs/coda/file.c
+++ b/fs/coda/file.c
@@ -80,12 +80,10 @@  coda_file_write_iter(struct kiocb *iocb, struct iov_iter *to)
 		goto finish_write;
 
 	inode_lock(coda_inode);
-	file_start_write(host_file);
 	ret = vfs_iter_write(cfi->cfi_container, to, &iocb->ki_pos, 0);
 	coda_inode->i_size = file_inode(host_file)->i_size;
 	coda_inode->i_blocks = (coda_inode->i_size + 511) >> 9;
 	inode_set_mtime_to_ts(coda_inode, inode_set_ctime_current(coda_inode));
-	file_end_write(host_file);
 	inode_unlock(coda_inode);
 
 finish_write:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 5d704461e3b4..35c9546b3396 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1186,9 +1186,7 @@  nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfsd_file *nf,
 	since = READ_ONCE(file->f_wb_err);
 	if (verf)
 		nfsd_copy_write_verifier(verf, nn);
-	file_start_write(file);
 	host_err = vfs_iter_write(file, &iter, &pos, flags);
-	file_end_write(file);
 	if (host_err < 0) {
 		commit_reset_write_verifier(nn, rqstp, host_err);
 		goto out_nfserr;
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 131621daeb13..690b173f34fc 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -436,9 +436,7 @@  static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter)
 	if (is_sync_kiocb(iocb)) {
 		rwf_t rwf = iocb_to_rw_flags(ifl);
 
-		file_start_write(real.file);
 		ret = vfs_iter_write(real.file, iter, &iocb->ki_pos, rwf);
-		file_end_write(real.file);
 		/* Update size */
 		ovl_file_modified(file);
 	} else {
diff --git a/fs/read_write.c b/fs/read_write.c
index 313f7eaaa9a7..87ca50f16a23 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -850,7 +850,7 @@  ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos,
 EXPORT_SYMBOL(vfs_iter_read);
 
 static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
-		loff_t *pos, rwf_t flags)
+			     loff_t *pos, rwf_t flags)
 {
 	size_t tot_len;
 	ssize_t ret = 0;
@@ -905,11 +905,18 @@  ssize_t vfs_iocb_iter_write(struct file *file, struct kiocb *iocb,
 EXPORT_SYMBOL(vfs_iocb_iter_write);
 
 ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos,
-		rwf_t flags)
+		       rwf_t flags)
 {
+	int ret;
+
 	if (!file->f_op->write_iter)
 		return -EINVAL;
-	return do_iter_write(file, iter, ppos, flags);
+
+	file_start_write(file);
+	ret = do_iter_write(file, iter, ppos, flags);
+	file_end_write(file);
+
+	return ret;
 }
 EXPORT_SYMBOL(vfs_iter_write);