On Thu, Mar 28, 2024 at 04:39:07PM -0400, Stefan Hajnoczi wrote:
> Signed-off-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> ---
> drivers/md/dm-linear.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
> index 2d3e186ca87e3..9b6cdfa4f951d 100644
> --- a/drivers/md/dm-linear.c
> +++ b/drivers/md/dm-linear.c
> @@ -147,6 +147,30 @@ static int linear_report_zones(struct dm_target *ti,
> #define linear_report_zones NULL
> #endif
>
> +static loff_t linear_seek_hole_data(struct dm_target *ti, loff_t offset,
> + int whence)
> +{
> + struct linear_c *lc = ti->private;
> + loff_t ti_begin = ti->begin << SECTOR_SHIFT;
> + loff_t ti_len = ti->len << SECTOR_SHIFT;
> + loff_t bdev_start = lc->start << SECTOR_SHIFT;
> + loff_t bdev_offset;
Okay, given my questions in 4/9, it looks like your intent is that
each callback for dm_seek_hole_data will obey its own ti-> limits.
> +
> + /* TODO underflow/overflow? */
> + bdev_offset = offset - ti_begin + bdev_start;
> +
> + bdev_offset = blkdev_seek_hole_data(lc->dev->bdev, bdev_offset,
> + whence);
> + if (bdev_offset < 0)
> + return bdev_offset;
> +
> + offset = bdev_offset - bdev_start;
> + if (offset >= ti_len)
> + return whence == SEEK_DATA ? -ENXIO : ti_begin + ti_len;
However, this is inconsistent with dm_blk_seek_hole_data_default in
4/9; I think you want to unconditionally return -ENXIO here, and let
the caller figure out when to turn -ENXIO back into end to proceed
with the next ti in the list.
OR, you may want to document the semantics that dm_seek_hole_data
callbacks must NOT return -ENXIO, but always return ti_begin + ti_len
when the answer (either SEEK_HOLE or SEEK_END) did not lie within the
current ti - it is DIFFERENT than the semantics for
blkdev_seek_hole_data, but gets normalized back into the expected
-ENXIO answer when dm_blk_do_seek_hole_data finally advances past the
last ti.
At any rate, I know this is an RFC series, but it goes to show that
comments will be essential, whichever interface you decide for
callbacks to honor (both a guarantee that callbacks will only ever see
SEEK_HOLE/SEEK_DATA in bounds, because earlier points in the call
stack have filtered out out-of-bounds and SEEK_SET; and constraints on
what the return value(s) must be for the various callbacks, especially
if it is different from the eventual return value of the overall
llseek syscall)
> +
> + return offset + ti_begin;
> +}
> +
> static int linear_iterate_devices(struct dm_target *ti,
> iterate_devices_callout_fn fn, void *data)
> {
> @@ -212,6 +236,7 @@ static struct target_type linear_target = {
> .direct_access = linear_dax_direct_access,
> .dax_zero_page_range = linear_dax_zero_page_range,
> .dax_recovery_write = linear_dax_recovery_write,
> + .seek_hole_data = linear_seek_hole_data,
> };
>
> int __init dm_linear_init(void)
> --
> 2.44.0
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
