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);
}
(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
