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
