| Message ID | 20231123092000.2665902-1-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi: target: core: add missing file_{start,end}_write() | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On 11/23/23 2:20 AM, Amir Goldstein wrote: > The callers of vfs_iter_write() are required to hold file_start_write(). > file_start_write() is a no-op for the S_ISBLK() case, but it is really > needed when the backing file is a regular file. > > We are going to move file_{start,end}_write() into vfs_iter_write(), but > we need to fix this first, so that the fix could be backported to stable > kernels. Reviewed-by: Jens Axboe <axboe@kernel.dk>
On Thu, Nov 23, 2023 at 10:04 PM Jens Axboe <axboe@kernel.dk> wrote: > > On 11/23/23 2:20 AM, Amir Goldstein wrote: > > The callers of vfs_iter_write() are required to hold file_start_write(). > > file_start_write() is a no-op for the S_ISBLK() case, but it is really > > needed when the backing file is a regular file. > > > > We are going to move file_{start,end}_write() into vfs_iter_write(), but > > we need to fix this first, so that the fix could be backported to stable > > kernels. > > Reviewed-by: Jens Axboe <axboe@kernel.dk> > Christian, Shall we just stash this at the bottom of vfs.rw and fixup "move file_{start,end}_write() into vfs_iter_write()" patch? I see no strong reason to expedite a fix for something rare that has been broken for a long time. If Martin decides to expedite it, we can alway rebase vfs.rw once the fix is merged to master. Thanks, Amir.
On Thu, 23 Nov 2023 11:20:00 +0200, Amir Goldstein wrote: > The callers of vfs_iter_write() are required to hold file_start_write(). > file_start_write() is a no-op for the S_ISBLK() case, but it is really > needed when the backing file is a regular file. > > We are going to move file_{start,end}_write() into vfs_iter_write(), but > we need to fix this first, so that the fix could be backported to stable > kernels. > > [...] Applied to the vfs.rw branch of the vfs/vfs.git tree. Patches in the vfs.rw branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.rw [1/1] scsi: target: core: add missing file_{start,end}_write() https://git.kernel.org/vfs/vfs/c/c85ff53e59e8
On Fri, Nov 24, 2023 at 09:54:49AM +0200, Amir Goldstein wrote: > On Thu, Nov 23, 2023 at 10:04 PM Jens Axboe <axboe@kernel.dk> wrote: > > > > On 11/23/23 2:20 AM, Amir Goldstein wrote: > > > The callers of vfs_iter_write() are required to hold file_start_write(). > > > file_start_write() is a no-op for the S_ISBLK() case, but it is really > > > needed when the backing file is a regular file. > > > > > > We are going to move file_{start,end}_write() into vfs_iter_write(), but > > > we need to fix this first, so that the fix could be backported to stable > > > kernels. > > > > Reviewed-by: Jens Axboe <axboe@kernel.dk> > > > > Christian, > > Shall we just stash this at the bottom of vfs.rw and fixup > "move file_{start,end}_write() into vfs_iter_write()" patch? Ok. > > I see no strong reason to expedite a fix for something rare > that has been broken for a long time. Agreed. > > If Martin decides to expedite it, we can alway rebase vfs.rw > once the fix is merged to master. It's now the first commit on that branch. Let me know if I should drop it.
Christian, >> If Martin decides to expedite it, we can alway rebase vfs.rw >> once the fix is merged to master. > > It's now the first commit on that branch. That's fine. Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c index 4d447520bab8..4e4cf6c34a77 100644 --- a/drivers/target/target_core_file.c +++ b/drivers/target/target_core_file.c @@ -332,11 +332,13 @@ static int fd_do_rw(struct se_cmd *cmd, struct file *fd, } iov_iter_bvec(&iter, is_write, bvec, sgl_nents, len); - if (is_write) + if (is_write) { + file_start_write(fd); ret = vfs_iter_write(fd, &iter, &pos, 0); - else + file_end_write(fd); + } else { ret = vfs_iter_read(fd, &iter, &pos, 0); - + } if (is_write) { if (ret < 0 || ret != data_length) { pr_err("%s() write returned %d\n", __func__, ret); @@ -467,7 +469,9 @@ fd_execute_write_same(struct se_cmd *cmd) } iov_iter_bvec(&iter, ITER_SOURCE, bvec, nolb, len); + file_start_write(fd_dev->fd_file); ret = vfs_iter_write(fd_dev->fd_file, &iter, &pos, 0); + file_end_write(fd_dev->fd_file); kfree(bvec); if (ret < 0 || ret != len) {
The callers of vfs_iter_write() are required to hold file_start_write(). file_start_write() is a no-op for the S_ISBLK() case, but it is really needed when the backing file is a regular file. We are going to move file_{start,end}_write() into vfs_iter_write(), but we need to fix this first, so that the fix could be backported to stable kernels. Suggested-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/ZV8ETIpM+wZa33B5@infradead.org/ Cc: stable@vger.kernel.org Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- Hi Martin, This bug is already fixed by commit "fs: move file_start_write() into vfs_iter_write()" on the vfs.rw branch in Christian's vfs tree, but Christoph suggested that I post a separate backportable fix for the scsi target code. You may decide if this is worth expediting to v6.7-rc or not. If not, then I think it would be best if Christian insert this patch at the bottom of the vfs.rw branch and revert in the later aformentioned commit. If you prefer to expedite it to v6.7-rc, then it's probably best to rebase vfs.rw branch after the fix hits master. Please let us know how you prefer to handle this patch. Thanks, Amir. drivers/target/target_core_file.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)