On 1/9/23 2:37?PM, David Howells wrote: > Jan Kara <jack@xxxxxxx> wrote: > >> So currently we already have BIO_NO_PAGE_REF flag and what you do in this >> patch partially duplicates that. So either I'd drop that flag or instead of >> bi_cleanup_mode variable (which honestly looks a bit wasteful given how we >> microoptimize struct bio) just add another BIO_ flag... > > I'm fine with translating the FOLL_* flags to the BIO_* flags. I could add a > BIO_PAGE_PINNED and translate: > > FOLL_GET => 0 > FOLL_PIN => BIO_PAGE_PINNED > 0 => BIO_NO_PAGE_REF > > It would seem that BIO_NO_PAGE_REF can't be set for BIO_PAGE_PINNED because > BIO_NO_PAGE_REF governs whether bio_release_pages() calls > __bio_release_pages() - which would be necessary. However, bio_release_page() > can do one or the other on the basis of BIO_PAGE_PINNED being specified. So > in my patch I would end up with: > > static void bio_release_page(struct bio *bio, struct page *page) > { > if (bio->bi_flags & BIO_NO_PAGE_REF) > ; > else if (bio->bi_flags & BIO_PAGE_PINNED) > unpin_user_page(page); > else > put_page(page); > } Let's please make this a bit more readable with: static void bio_release_page(struct bio *bio, struct page *page) { if (bio->bi_flags & BIO_NO_PAGE_REF) return; if (bio->bi_flags & BIO_PAGE_PINNED) unpin_user_page(page); else put_page(page); } -- Jens Axboe