| Message ID | 20230424054926.26927-15-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/17] fs: unexport buffer_check_dirty_writeback | expand |
On Mon, Apr 24, 2023 at 07:49:23AM +0200, Christoph Hellwig wrote: > Open code __generic_file_write_iter to remove the indirect call into > ->direct_IO and to prepare using the iomap based write code. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/fops.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 44 insertions(+), 2 deletions(-) > > diff --git a/block/fops.c b/block/fops.c > index b670aa7c5bb745..fd510b6142bd57 100644 > --- a/block/fops.c > +++ b/block/fops.c > @@ -508,6 +508,29 @@ static int blkdev_close(struct inode *inode, struct file *filp) > return 0; > } > > +static ssize_t > +blkdev_direct_write(struct kiocb *iocb, struct iov_iter *from) > +{ > + size_t count = iov_iter_count(from); > + ssize_t written; > + > + written = kiocb_invalidate_pages(iocb, count); > + if (written) { > + if (written == -EBUSY) > + return 0; > + return written; > + } > + > + written = blkdev_direct_IO(iocb, from); > + if (written > 0) { > + kiocb_invalidate_post_write(iocb, count); > + iocb->ki_pos += written; > + } Written can be negative here after blkdev_direct_IO() > + if (written != -EIOCBQUEUED) > + iov_iter_revert(from, count - written - iov_iter_count(from)); And we'll then use it here on iov_iter_revert() and this can crash on with some values. For example this can crash on a 4k write attempt on a 32k drive when experimenting wit large block sizes. kernel BUG at lib/iov_iter.c:999! invalid opcode: 0000 [#1] PREEMPT SMP PTI CPU: 4 PID: 949 Comm: fio Not tainted 6.3.0-large-block-20230426-dirty#28 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-debian-1.16.0-5 04/01/2014 +RIP: 0010:iov_iter_revert.part.0+0x16e/0x170 Code: f9 40 a2 63 af 74 07 03 56 08 89 d8 29 d0 89 45 08 44 89 6d 20 <etc> RSP: 0018:ffffaa52006cfc60 EFLAGS: 00010246 RAX: 0000000000000016 RBX: 0000000000000016 RCX: 0000000000000000 RDX: 0000000000000004 RSI: 0000000000000006 RDI: ffffaa52006cfd08 RBP: ffffaa52006cfd08 R08: 0000000000000000 R09: ffffaa52006cfb40 R10: 0000000000000003 R11: ffffffffafcc21e8 R12: 0000000000004000 R13: 0000000000003fea R14: ffff9de3d7565e00 R15: ffff9de3c1f68600 FS: 00007f8bfe726c40(0000) GS:ffff9de43bd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f8bf5eadd68 CR3: 0000000102c76001 CR4: 0000000000770ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: <TASK> blkdev_direct_write+0xf0/0x160 blkdev_write_iter+0x11b/0x230 io_write+0x10c/0x420 ? kmem_cache_alloc_bulk+0x2a1/0x410 ? fget+0x79/0xb0 io_issue_sqe+0x60/0x3b0 ? io_prep_rw+0x5a/0x190 io_submit_sqes+0x1e6/0x640 __do_sys_io_uring_enter+0x54c/0xb90 ? handle_mm_fault+0x9a/0x340 ? preempt_count_add+0x47/0xa0 ? up_read+0x37/0x70 ? do_user_addr_fault+0x27c/0x780 do_syscall_64+0x37/0x90 entry_SYSCALL_64_after_hw Although I fixed it with an early check on this routine with: if (count < bdev_logical_block_size(bdev)) return -EINVAL; I think this can just be fixed by also using the alignment check earier here: if (blkdev_dio_unaligned(bdev, pos, iter)) return -EINVAL; Luis
diff --git a/block/fops.c b/block/fops.c index b670aa7c5bb745..fd510b6142bd57 100644 --- a/block/fops.c +++ b/block/fops.c @@ -508,6 +508,29 @@ static int blkdev_close(struct inode *inode, struct file *filp) return 0; } +static ssize_t +blkdev_direct_write(struct kiocb *iocb, struct iov_iter *from) +{ + size_t count = iov_iter_count(from); + ssize_t written; + + written = kiocb_invalidate_pages(iocb, count); + if (written) { + if (written == -EBUSY) + return 0; + return written; + } + + written = blkdev_direct_IO(iocb, from); + if (written > 0) { + kiocb_invalidate_post_write(iocb, count); + iocb->ki_pos += written; + } + if (written != -EIOCBQUEUED) + iov_iter_revert(from, count - written - iov_iter_count(from)); + return written; +} + /* * Write data to the block device. Only intended for the block device itself * and the raw driver which basically is a fake block device. @@ -517,7 +540,8 @@ static int blkdev_close(struct inode *inode, struct file *filp) */ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) { - struct block_device *bdev = iocb->ki_filp->private_data; + struct file *file = iocb->ki_filp; + struct block_device *bdev = file->private_data; struct inode *bd_inode = bdev->bd_inode; loff_t size = bdev_nr_bytes(bdev); size_t shorted = 0; @@ -538,13 +562,31 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from) if ((iocb->ki_flags & (IOCB_NOWAIT | IOCB_DIRECT)) == IOCB_NOWAIT) return -EOPNOTSUPP; + ret = file_remove_privs(file); + if (ret) + return ret; + + ret = file_update_time(file); + if (ret) + return ret; + size -= iocb->ki_pos; if (iov_iter_count(from) > size) { shorted = iov_iter_count(from) - size; iov_iter_truncate(from, size); } - ret = __generic_file_write_iter(iocb, from); + current->backing_dev_info = bdev->bd_disk->bdi; + if (iocb->ki_flags & IOCB_DIRECT) { + ret = blkdev_direct_write(iocb, from); + if (ret >= 0 && iov_iter_count(from)) + ret = direct_write_fallback(iocb, from, ret, + generic_perform_write(iocb, from)); + } else { + ret = generic_perform_write(iocb, from); + } + current->backing_dev_info = NULL; + if (ret > 0) ret = generic_write_sync(iocb, ret); iov_iter_reexpand(from, iov_iter_count(from) + shorted);
Open code __generic_file_write_iter to remove the indirect call into ->direct_IO and to prepare using the iomap based write code. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/fops.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-)