On Mar 09, 2021 / 17:28, Kanchan Joshi wrote: > On Tue, Mar 9, 2021 at 4:36 PM Damien Le Moal <Damien.LeMoal@xxxxxxx> wrote: > > > > On 2021/03/09 14:49, Kanchan Joshi wrote: > > > On Mon, Mar 8, 2021 at 2:11 PM Shin'ichiro Kawasaki > > > <shinichiro.kawasaki@xxxxxxx> wrote: > > >> > > >> When zone reset ioctl and data read race for a same zone on zoned block > > >> devices, the data read leaves stale page cache even though the zone > > >> reset ioctl zero clears all the zone data on the device. To avoid > > >> non-zero data read from the stale page cache after zone reset, discard > > >> page cache of reset target zones. In same manner as fallocate, call the > > >> function truncate_bdev_range() in blkdev_zone_mgmt_ioctl() before and > > >> after zone reset to ensure the page cache discarded. > > >> > > >> This patch can be applied back to the stable kernel version v5.10.y. > > >> Rework is needed for older stable kernels. > > >> > > >> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> > > >> Fixes: 3ed05a987e0f ("blk-zoned: implement ioctls") > > >> Cc: <stable@xxxxxxxxxxxxxxx> # 5.10+ > > >> --- > > >> block/blk-zoned.c | 30 ++++++++++++++++++++++++++++-- > > >> 1 file changed, 28 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/block/blk-zoned.c b/block/blk-zoned.c > > >> index 833978c02e60..990a36be2927 100644 > > >> --- a/block/blk-zoned.c > > >> +++ b/block/blk-zoned.c > > >> @@ -329,6 +329,9 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, > > >> struct request_queue *q; > > >> struct blk_zone_range zrange; > > >> enum req_opf op; > > >> + sector_t capacity; > > >> + loff_t start, end; > > >> + int ret; > > >> > > >> if (!argp) > > >> return -EINVAL; > > >> @@ -349,9 +352,22 @@ int blkdev_zone_mgmt_ioctl(struct block_device *bdev, fmode_t mode, > > >> if (copy_from_user(&zrange, argp, sizeof(struct blk_zone_range))) > > >> return -EFAULT; > > >> > > >> + capacity = get_capacity(bdev->bd_disk); > > >> + if (zrange.sector + zrange.nr_sectors <= zrange.sector || > > >> + zrange.sector + zrange.nr_sectors > capacity) > > >> + /* Out of range */ > > >> + return -EINVAL; > > >> + > > >> + start = zrange.sector << SECTOR_SHIFT; > > >> + end = ((zrange.sector + zrange.nr_sectors) << SECTOR_SHIFT) - 1; > > > > > > How about doing all this calculation only when it is applicable i.e. > > > only for reset-zone case, and not for other cases (open/close/finish > > > zone). > > > > > > Also apart from "out of range" (which is covered here), there are few > > > more cases when blkdev_zone_mgmt() may fail it (not covered here). > > > Perhaps the whole pre and post truncate part can fit better inside > > > blkdev_zone_mgmt itself. > > > > No, I do not think so. That would add overhead for in-kernel users of zone reset > > for no good reason since these would typically take care of cached pages > > themselves (e.g. FS) and would not trigger page caching using the bdev inode anyway. > > Agreed. In that case moving the pre-truncate processing from > common-path to under BLKRESETZONE will suffice. > With that refactoring in place, it looks good. > > Reviewed-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx> Thanks. Will reflect your comment and the other comment by Damien for v2. -- Best Regards, Shin'ichiro Kawasaki