On Thu, Jan 22, 2026 at 04:01:13PM -0800, Darrick J. Wong wrote:
> > /**
> > * bio_integrity_prep - Prepare bio for integrity I/O
> > * @bio: bio to prepare
> > + * @action: preparation action needed
>
> What is @action?
Yes.
> Is it a bitset of BI_ACT_* values? If yes, then can
> the comment please say that explicitly?
Is this good enough?
* @action: preparation action needed (BI_ACT_*)
> > +static bool bi_offload_capable(struct blk_integrity *bi)
> > +{
> > + return bi->metadata_size == bi->pi_tuple_size;
> > +}
>
> Just out of curiosity, what happens if metadata_size > pi_tuple_size?
Then we still have to provide a buffer as the automatic insert/strip
doesn't work. (I find the offload name rather confusing for this)
> Can it be the case that metadata_size < pi_tuple_size?
No. See blk_validate_integrity_limits:
if (bi->pi_offset + bi->pi_tuple_size > bi->metadata_size) {
pr_warn("pi_offset (%u) + pi_tuple_size (%u) exceeds
metadata_size (%u)\n",
bi->pi_offset, bi->pi_tuple_size,
bi->metadata_size);
return -EINVAL;
}
>
> > +unsigned int __bio_integrity_action(struct bio *bio)
>
> Hrm, this function returns a bitset of BI_ACT_* flags, doesn't it?
>
> Would be kinda nice if a comment could say that.
Is this ok?
/**
* bio_integrity_action - return the integrity action needed for a bio
* @bio: bio to operate on
*
* Returns the mask of integrity actions (BI_ACT_*) that need to be performed
* for @bio.
*/
> > + /*
> > + * Zero the memory allocated to not leak uninitialized kernel
> > + * memory to disk for non-integrity metadata where nothing else
> > + * initializes the memory.
>
> Er... does someone initialize it eventually? Such as the filesystem?
> Or maybe an io_uring caller?
For integrity metadata? The code called later fills it out. But it
doesn't fill non-integrity metadata, so we need to zero it.
> > + */
> > + if (bi->flags & BLK_INTEGRITY_NOGENERATE) {
> > + if (bi_offload_capable(bi))
> > + return 0;
> > + return BI_ACT_BUFFER | BI_ACT_ZERO;
> > + }
> > +
> > + if (bi->metadata_size > bi->pi_tuple_size)
> > + return BI_ACT_BUFFER | BI_ACT_CHECK | BI_ACT_ZERO;
> > + return BI_ACT_BUFFER | BI_ACT_CHECK;
>
> "check" feels like a weird name for a write, where we're generating the
> PI information. It really means "block layer takes care of PI
> generation and validation", right? As opposed to whichever upper layer
> is using the block device?
>
> BI_ACT_YOUDOIT <snerk>
>
> How about BI_ACT_BDEV /* block layer checks/validates PI */
I think BI_ACT_BDEV is not very useful. Check is supposed to
include generate and verify, but I'm not sure how we could word this
in a nice way.
