mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Damien Le Moal <dlemoal@kernel.org>
To: Shashank Mohan Jain <jain.sm@gmail.com>, Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@lst.de>,
	Johannes Thumshirn <johannes.thumshirn@wdc.com>,
	Hannes Reinecke <hare@suse.de>,
	Chaitanya Kulkarni <kch@nvidia.com>,
	"Martin K. Petersen" <mkp@kernel.org>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] block: only use the cached zone report if BLK_ZONE_REP_CACHED is set
Date: Sun, 11 Oct 2026 16:46:15 +0200	[thread overview]
Message-ID: <e146448b-5210-4215-821f-181c91625141@kernel.org> (raw)
In-Reply-To: <20261011051906.60397-2-jain.sm@gmail.com>

On 2026/10/11 7:19, Shashank Mohan Jain wrote:
> BLKREPORTZONEV2 takes the flags field of struct blk_zone_report as an
> input. With BLK_ZONE_REP_CACHED the report is built from the zone
> information cached by the block layer. Without it, BLKREPORTZONEV2 is
> documented to behave like BLKREPORTZONE and to get the report from the
> device: this is what the changelog of the commit that added the ioctl
> and the comments in include/uapi/linux/blkzoned.h say.
> 
> blkdev_report_zones_ioctl() only uses the flag to validate the input
> and calls blkdev_report_zones_cached() for every BLKREPORTZONEV2
> request. A caller that passes flags == 0 gets the cached report, in
> which implicitly open, explicitly open and closed zones all have the
> condition BLK_ZONE_COND_ACTIVE, and an explicitly opened empty zone is
> reported as empty.
> 
> Example with a zoned null_blk device (10 MiB, 4 MiB zones) after
> BLKOPENZONE on zone 0, and a write of 8 sectors to zone 1 followed by
> BLKCLOSEZONE:
> 
>   BLKREPORTZONE:             zone 0 cond 0x3 (EXP_OPEN)
>                              zone 1 cond 0x4 (CLOSED)
>   BLKREPORTZONEV2, flags 0:  zone 0 cond 0x1 (EMPTY)
>                              zone 1 cond 0xff (ACTIVE)
> 
> The uapi header marks BLKREPORTZONE as deprecated in favour of
> BLKREPORTZONEV2. A program that follows this and does not ask for
> cached information can no longer tell open zones from closed ones, and
> is handed BLK_ZONE_COND_ACTIVE, which is only defined for the cached
> report.
> 
> Call blkdev_report_zones_cached() only if BLK_ZONE_REP_CACHED is set,
> and blkdev_report_zones() otherwise.
> 
> Tested with zoned null_blk devices in qemu: a program that compares
> the reports of BLKREPORTZONE and BLKREPORTZONEV2 with flags 0 after
> the sequence above finds the differences before this change and none
> with it. The report with BLK_ZONE_REP_CACHED is unchanged.
> 
> Fixes: b30ffcdc0c15 ("block: introduce BLKREPORTZONESV2 ioctl")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
> ---
> Prepared with Claude Code (Anthropic), model Claude Opus 5.5
> (claude-opus-5-5).
> 
>  block/blk-zoned.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 475aa16bc4..9555eae91a 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -400,6 +400,13 @@ int blkdev_report_zones_ioctl(struct block_device *bdev, unsigned int cmd,
>  	case BLKREPORTZONEV2:
>  		if (rep.flags & ~BLK_ZONE_REPV2_INPUT_FLAGS)
>  			return -EINVAL;
> +		if (!(rep.flags & BLK_ZONE_REP_CACHED)) {
> +			ret = blkdev_report_zones(bdev, rep.sector,
> +						  rep.nr_zones,
> +						  blkdev_copy_zone_to_user,
> +						  &args);
> +			break;
> +		}
>  		ret = blkdev_report_zones_cached(bdev, rep.sector, rep.nr_zones,
>  					 blkdev_copy_zone_to_user, &args);

Can you reverse this if to be "if (rep.flags & BLK_ZONE_REP_CACHED) {" and to
call blkdev_report_zones_cached if the condition is true? That would be a lot
more logical.

Also, the condition should probably be:

	if ((rep.flags & BLK_ZONE_REP_CACHED) &&
	    blkdev_has_cached_report_zones(bdev))

to be more efficient and avoid a call to blkdev_report_zones_cached() which will
turn into a regular device report zones.


-- 
Damien Le Moal
Western Digital Research

  reply	other threads:[~2026-10-11 14:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-11  5:19 [PATCH 0/2] block: two fixes for the zone report ioctls Shashank Mohan Jain
2026-10-11  5:19 ` [PATCH 1/2] block: only use the cached zone report if BLK_ZONE_REP_CACHED is set Shashank Mohan Jain
2026-10-11 14:46   ` Damien Le Moal [this message]
2026-10-11 15:44     ` shashank Jain
2026-10-11  5:19 ` [PATCH 2/2] block: fix the length of a smaller last zone in blkdev_get_zone_info() Shashank Mohan Jain
2026-10-11 14:47   ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e146448b-5210-4215-821f-181c91625141@kernel.org \
    --to=dlemoal@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=jain.sm@gmail.com \
    --cc=johannes.thumshirn@wdc.com \
    --cc=kch@nvidia.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkp@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®