Re: [PATCH 2/2] block: pass a phys_addr_t to get_max_segment_size

Chaitanya Kulkarni
2024-07-06T10:37:16Z

On 7/6/2024 12:52 AM, Christoph Hellwig wrote:
> Work on a single address to simplify the logic, and prepare the callers
> from using better helpers.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
> ---
> block/blk-merge.c | 25 +++++++++++--------------
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index cff20bcc0252a7..e41ea331809936 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -209,23 +209,22 @@ static inline unsigned get_max_io_size(struct bio *bio,
> /**
> * get_max_segment_size() - maximum number of bytes to add as a single segment
> * @lim: Request queue limits.
> - * @start_page: See below.
> - * @offset: Offset from @start_page where to add a segment.
> + * @paddr: address of the range to add
> + * @max_len: maximum length available to add at @paddr
> *
> - * Returns the maximum number of bytes that can be added as a single segment.
> + * Returns the maximum number of bytes of the range starting at @paddr that can
> + * be added to a single segment.
> */
> static inline unsigned get_max_segment_size(const struct queue_limits *lim,
> - struct page *start_page, unsigned long offset)
> + phys_addr_t paddr, unsigned int len)
> {
> - unsigned long mask = lim->seg_boundary_mask;
> -
> - offset = mask & (page_to_phys(start_page) + offset);
> -
> /*
> * Prevent an overflow if mask = ULONG_MAX and offset = 0 by adding 1
> * after having calculated the minimum.
> */
> - return min(mask - offset, (unsigned long)lim->max_segment_size - 1) + 1;
> + return min_t(unsigned long, len,
> + min(lim->seg_boundary_mask - (lim->seg_boundary_mask & paddr),
> + (unsigned long)lim->max_segment_size - 1) + 1);
> }
>

Looks good, is it possible to re-write last
return min_t(..., ..., min(..., ...)) statement something like totally
untested in [1] ?

Irrespective of that, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

[1]

static inline unsigned get_max_segment_size(const struct queue_limits *lim,
phys_addr_t paddr, unsigned int paddr_len)
{
unsigned long paddr_max_seg_allowed_len;
unsigned long paddr_seg_boundry;

paddr_seg_boundry =
lim->seg_boundary_mask - (lim->seg_boundary_mask & paddr);
/*
* Prevent an overflow if mask = ULONG_MAX and offset = 0 by
adding 1
* after having calculated the minimum.
*/
paddr_max_seg_allowed_len = min(paddr_seg_boundry,
(unsigned long)lim->max_segment_size -
1) + 1;
return min_t(unsigned long, paddr_len, paddr_max_seg_allowed_len);
}