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

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

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);
}