On Wed, Jan 08, 2020 at 06:02:48AM -0800, Christoph Hellwig wrote:
> > +static inline unsigned get_max_segment_size(const struct request_queue *q,
> > + const struct page *start_page,
> > + unsigned long offset)
> > {
> > unsigned long mask = queue_segment_boundary(q);
> >
> > - /* default segment boundary mask means no boundary limit */
> > - if (mask == BLK_SEG_BOUNDARY_MASK)
> > - return queue_max_segment_size(q);
> > -
> > - return min_t(unsigned long, mask - (mask & offset) + 1,
> > + offset = mask & (page_to_phys(start_page) + offset);
>
> This looks weird and potentionaly incorrect - once you add the offset
> to page_phys it really isn't an offset anymore and should be in a
> variable named paddr or similar. And that needs to use a phys_addr_t
It can be thought as offset from viewpoint of segment boundary.
> as we can have 32-bit architectures that use 64-bit physical addresses.
>
> I'd also pass in the actual phys_addr_t instead of the page and offset.
>
It has been addressed in:
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=block-5.5&id=ecd255974caa45901d0b8fab03626e0a18fbc81a
Thanks,
Ming
