Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Thu, 29 Feb 2024 15:40:09 -0500 | Subject | Re: [PATCH] block: Fix page refcounts for unaligned buffers in __bio_release_pages() | From | Tony Battersby <> |
| |
On 2/29/24 14:53, Matthew Wilcox wrote: > On Thu, Feb 29, 2024 at 01:08:09PM -0500, Tony Battersby wrote: >> Fix an incorrect number of pages being released for buffers that do not >> start at the beginning of a page. > Oh, I see what I did. Wouldn't a simpler fix be to just set "done" to > offset_in_page(fi.offset)?
Actually it would be:
ssize_t done = -offset_in_page(offset);
But then you have signed vs. unsigned comparison in the while(), or you could rearrange the loop to avoid the negative, but then it gets clunky.
> >> @@ -1152,7 +1152,7 @@ void __bio_release_pages(struct bio *bio, bool mark_dirty) >> >> bio_for_each_folio_all(fi, bio) { >> struct page *page; >> - size_t done = 0; >> + size_t nr_pages; >> >> if (mark_dirty) { >> folio_lock(fi.folio); >> @@ -1160,10 +1160,11 @@ void __bio_release_pages(struct bio *bio, bool mark_dirty) >> folio_unlock(fi.folio); >> } >> page = folio_page(fi.folio, fi.offset / PAGE_SIZE); >> + nr_pages = (fi.offset + fi.length - 1) / PAGE_SIZE - >> + fi.offset / PAGE_SIZE + 1; >> do { >> bio_release_page(bio, page++); >> - done += PAGE_SIZE; >> - } while (done < fi.length); >> + } while (--nr_pages != 0); >> } >> } >> EXPORT_SYMBOL_GPL(__bio_release_pages); > The long-term path here, I think, is to replace this bio_release_page() > with a bio_release_folio(folio, offset, length) which calls into > a new unpin_user_folio(folio, nr) which calls gup_put_folio().
I developed the patch with the 6.1 stable series, which just has:
nr_pages = (fi.offset + fi.length - 1) / PAGE_SIZE - fi.offset / PAGE_SIZE + 1; folio_put_refs(fi.folio, nr_pages);
Which is another reason that I went for the direct nr_pages calculation. Would you still prefer the negative offset_in_page() approach?
Tony
| ![\](/images/icornerr.gif) |