Messages in this thread | | | From | David Howells <> | Subject | Re: [PATCH v4 7/7] iov_iter, block: Make bio structs pin pages rather than ref'ing if appropriate | Date | Mon, 09 Jan 2023 21:37:35 +0000 |
| |
Jan Kara <jack@suse.cz> 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); }
(This is called from four places, so it has to handle BIO_NO_PAGE_REF).
It might make sense flip the logic of BIO_NO_PAGE_REF so that we have, say:
FOLL_GET => BIO_PAGE_REFFED FOLL_PIN => BIO_PAGE_PINNED 0 => 0
Set BIO_PAGE_REFFED by default and clear it in bio_iov_bvec_set().
Note that one reason I was thinking of saving the returned FOLL_* flags is that I don't know if, at some point, the VM will acquire yet more different cleanup modes - or even if a page could at some point be both ref'd *and* pinned.
Also, I could change the interface to return something other than FOLL_* - it just seems that they're appropriate given the underlying VM interface.
David
| |