On Wed, Oct 19, 2022 at 03:23:17PM -0700, Bart Van Assche wrote:
> This patch removes a conditional jump from get_max_segment_size(). The
> x86-64 assembler code for this function without this patch is as follows:
>
> 206 return min_not_zero(mask - offset + 1,
> 0x0000000000000118 <+72>: not %rax
> 0x000000000000011b <+75>: and 0x8(%r10),%rax
> 0x000000000000011f <+79>: add $0x1,%rax
> 0x0000000000000123 <+83>: je 0x138 <bvec_split_segs+104>
> 0x0000000000000125 <+85>: cmp %rdx,%rax
> 0x0000000000000128 <+88>: mov %rdx,%r12
> 0x000000000000012b <+91>: cmovbe %rax,%r12
> 0x000000000000012f <+95>: test %rdx,%rdx
> 0x0000000000000132 <+98>: mov %eax,%edx
> 0x0000000000000134 <+100>: cmovne %r12d,%edx
>
> With this patch applied:
>
> 206 return min(mask - offset, (unsigned long)lim->max_segment_size - 1) + 1;
> 0x000000000000003f <+63>: mov 0x28(%rdi),%ebp
> 0x0000000000000042 <+66>: not %rax
> 0x0000000000000045 <+69>: and 0x8(%rdi),%rax
> 0x0000000000000049 <+73>: sub $0x1,%rbp
> 0x000000000000004d <+77>: cmp %rbp,%rax
> 0x0000000000000050 <+80>: cmova %rbp,%rax
> 0x0000000000000054 <+84>: add $0x1,%eax
>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Keith Busch <kbusch@xxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> block/blk-merge.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/block/blk-merge.c b/block/blk-merge.c
> index 58fdc3f8905b..35a8f75cc45d 100644
> --- a/block/blk-merge.c
> +++ b/block/blk-merge.c
> @@ -186,6 +186,14 @@ static inline unsigned get_max_io_size(struct bio *bio,
> return max_sectors & ~(lbs - 1);
> }
>
> +/**
> + * 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.
> + *
> + * Returns the maximum number of bytes that can be added as a single segment.
> + */
> static inline unsigned get_max_segment_size(const struct queue_limits *lim,
> struct page *start_page, unsigned long offset)
> {
> @@ -194,11 +202,10 @@ static inline unsigned get_max_segment_size(const struct queue_limits *lim,
> offset = mask & (page_to_phys(start_page) + offset);
>
> /*
> - * overflow may be triggered in case of zero page physical address
> - * on 32bit arch, use queue's max segment size when that happens.
> + * Prevent an overflow if mask = ULONG_MAX and offset = 0 by adding 1
> + * after having calculated the minimum.
> */
> - return min_not_zero(mask - offset + 1,
> - (unsigned long)lim->max_segment_size);
> + return min(mask - offset, (unsigned long)lim->max_segment_size - 1) + 1;
> }
Looks fine,
Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx>
Thanks,
Ming
