| Message ID | 2ef85c782997ad40e923e7640039e0c7795e19da.1724297388.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implement async block discards/etc. via io_uring | expand |
On Thu, Aug 22, 2024 at 04:35:54AM +0100, Pavel Begunkov wrote: > In preparation to further changes extract a helper function out of > blk_ioctl_discard() that validates if it's allowed to do a write-like > operation for the given range. This isn't about a write, it is about a discard.
On 8/22/24 07:33, Christoph Hellwig wrote: > On Thu, Aug 22, 2024 at 04:35:54AM +0100, Pavel Begunkov wrote: >> In preparation to further changes extract a helper function out of >> blk_ioctl_discard() that validates if it's allowed to do a write-like >> operation for the given range. > > This isn't about a write, it is about a discard. It's used for other commands in the series, all of them are semantically "writes", or modifying data operations if that's better. How would you call it? Some blk_modify_validate_args, maybe?
On Thu, Aug 22, 2024 at 01:36:10PM +0100, Pavel Begunkov wrote: > It's used for other commands in the series, all of them are > semantically "writes", or modifying data operations if that's > better. How would you call it? Some blk_modify_validate_args, > maybe? Maybe just split the writability checks out and rename the rest blk_validate_byte_range, that way the usage is pretty clearly defined. Bonus points for writing a kerneldoc other other comment header explaining it.
diff --git a/block/ioctl.c b/block/ioctl.c index e8e4a4190f18..8df0bc8002f5 100644 --- a/block/ioctl.c +++ b/block/ioctl.c @@ -92,38 +92,49 @@ static int compat_blkpg_ioctl(struct block_device *bdev, } #endif +static int blk_validate_write(struct block_device *bdev, blk_mode_t mode, + uint64_t start, uint64_t len) +{ + unsigned int bs_mask; + uint64_t end; + + if (!(mode & BLK_OPEN_WRITE)) + return -EBADF; + if (bdev_read_only(bdev)) + return -EPERM; + + bs_mask = bdev_logical_block_size(bdev) - 1; + if ((start | len) & bs_mask) + return -EINVAL; + if (!len) + return -EINVAL; + + if (check_add_overflow(start, len, &end) || end > bdev_nr_bytes(bdev)) + return -EINVAL; + + return 0; +} + static int blk_ioctl_discard(struct block_device *bdev, blk_mode_t mode, unsigned long arg) { - unsigned int bs_mask = bdev_logical_block_size(bdev) - 1; - uint64_t range[2], start, len, end; + uint64_t range[2], start, len; struct bio *prev = NULL, *bio; sector_t sector, nr_sects; struct blk_plug plug; int err; - if (!(mode & BLK_OPEN_WRITE)) - return -EBADF; - - if (!bdev_max_discard_sectors(bdev)) - return -EOPNOTSUPP; - if (bdev_read_only(bdev)) - return -EPERM; - if (copy_from_user(range, (void __user *)arg, sizeof(range))) return -EFAULT; - start = range[0]; len = range[1]; - if (!len) - return -EINVAL; - if ((start | len) & bs_mask) - return -EINVAL; + if (!bdev_max_discard_sectors(bdev)) + return -EOPNOTSUPP; - if (check_add_overflow(start, len, &end) || - end > bdev_nr_bytes(bdev)) - return -EINVAL; + err = blk_validate_write(bdev, mode, start, len); + if (err) + return err; filemap_invalidate_lock(bdev->bd_mapping); err = truncate_bdev_range(bdev, mode, start, start + len - 1);
In preparation to further changes extract a helper function out of blk_ioctl_discard() that validates if it's allowed to do a write-like operation for the given range. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- block/ioctl.c | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-)