Re: [PATCH] block: fix adding folio to bio

[Ming Lei <ming.lei@redhat.com>]

On Wed, Mar 12, 2025 at 12:19:12PM +0000, Matthew Wilcox wrote:
> On Wed, Mar 12, 2025 at 07:38:05PM +0800, Ming Lei wrote:
> > +++ b/block/bio.c
> > @@ -1026,9 +1026,17 @@ EXPORT_SYMBOL(bio_add_page);
> >  void bio_add_folio_nofail(struct bio *bio, struct folio *folio, size_t len,
> >  			  size_t off)
> >  {
> > +	struct page *page = &folio->page;
> > +
> >  	WARN_ON_ONCE(len > UINT_MAX);
> > -	WARN_ON_ONCE(off > UINT_MAX);
> > -	__bio_add_page(bio, &folio->page, len, off);
> > +	if (unlikely(off > UINT_MAX)) {
> 
> I think we should probably make this:
> 
> 	if (unlikely(off + len > UINT_MAX))
> 
> because I'm not sure that everything will cope well with an I/O that
> crosses the 4GB boundary.

If hardware doesn't support it, the bio will be splitted before submitting
to the disk, so I think the check isn't needed here.

> 
> Actually, why bother with the conditional?  Let's just do it always.
> 
>  {
> +	unsigned long nr = off / PAGE_SIZE;
>  	WARN_ON_ONCE(len > UINT_MAX);
> -	WARN_ON_ONCE(off > UINT_MAX);
> -	__bio_add_page(bio, &folio->page, len, off);
> +	off = off % PAGE_SIZE;
> +	__bio_add_page(bio, folio_page(folio, nr), len, off);
>  }
> 
> Also you need to do bio_add_folio(), not just the _nofail variant.
 
OK, will cover bio_add_folio() in V2 by the unconditional way.


Thanks,
Ming