On Tue, Mar 12, 2024 at 11:22:53AM -0400, Mike Snitzer wrote:
> 4) blk_validate_limits() will reject the limits that
> blk_stack_limits() created:
> /*
> * Devices that require a virtual boundary do not support scatter/gather
> * I/O natively, but instead require a descriptor list entry for each
> * page (which might not be identical to the Linux PAGE_SIZE). Because
> * of that they are not limited by our notion of "segment size".
> */
> if (lim->virt_boundary_mask) {
> if (WARN_ON_ONCE(lim->max_segment_size &&
> lim->max_segment_size != UINT_MAX))
> return -EINVAL;
> lim->max_segment_size = UINT_MAX;
> } else {
> /*
> * The maximum segment size has an odd historic 64k default that
> * drivers probably should override. Just like the I/O size we
> * require drivers to at least handle a full page per segment.
> */
> if (!lim->max_segment_size)
> lim->max_segment_size = BLK_MAX_SEGMENT_SIZE;
> if (WARN_ON_ONCE(lim->max_segment_size < PAGE_SIZE))
> return -EINVAL;
> }
>
> blk_validate_limits() is currently very pedantic. I discussed with Jens
> briefly and we're thinking it might make sense for blk_validate_limits()
> to be more forgiving by _not_ imposing hard -EINVAL failure. That in
> the interim, during this transition to more curated and atomic limits, a
> WARN_ON_ONCE() splat should serve as enough notice to developers (be it
> lower level nvme or higher-level virtual devices like DM).
>
> BUT for this specific max_segment_size case, the constraints of dm-crypt
> are actually more conservative due to crypto requirements. Yet nvme's
> more general "don't care, but will care if non-nvme driver does" for
> this particular max_segment_size limit is being imposed when validating
> the combined limits that dm-crypt will impose at the top-level.
>
> All said, the above "if (lim->virt_boundary_mask)" check in
> blk_validate_limits() looks bogus for stacked device limits.
Yes, I think you're right. I can't tell why this check makes sense for
any device, not just stacked ones. It could verify lim->max_segment_size
is >= virt_boundary_mask, but to require it be UINT_MAX doesn't look
necessary.
