On Mon 15-04-19 20:22:04, Jerome Glisse wrote: > On Mon, Apr 15, 2019 at 04:59:52PM +0200, Jan Kara wrote: > > Hi Jerome! > > > > On Thu 11-04-19 17:08:29, jglisse@xxxxxxxxxx wrote: > > > From: Jérôme Glisse <jglisse@xxxxxxxxxx> > > > > > > We want to keep track of how we got a reference on page added to bio_vec > > > ie wether the page was reference through GUP (get_user_page*) or not. So > > > add a flag to bio_add_page()/bio_add_pc_page()/__bio_add_page() to that > > > effect. > > > > Thanks for writing this patch set! Looking through patches like this one, > > I'm a bit concerned. With so many bio_add_page() callers it's difficult to > > get things right and not regress in the future. I'm wondering whether the > > things won't be less error-prone if we required that all page reference > > from bio are gup-like (not necessarily taken by GUP, if creator of the bio > > gets to struct page he needs via some other means (e.g. page cache lookup), > > he could just use get_gup_pin() helper we'd provide). After all, a page > > reference in bio means that the page is pinned for the duration of IO and > > can be DMAed to/from so it even makes some sense to track the reference > > like that. Then bio_put() would just unconditionally do put_user_page() and > > we won't have to propagate the information in the bio. > > > > Do you think this would be workable and easier? > > Thinking again on this, i can drop that patch and just add a new > bio_add_page_from_gup() and then it would be much more obvious that > only very few places need to use that new version and they are mostly > obvious places. It is usualy GUP then right away add the pages to bio > or bvec. Yes, that's another option. Probably second preferred by me after my own proposal ;) > We can probably add documentation around GUP explaining that if you > want to build a bio or bvec from GUP you must pay attention to which > function you use. Yes, although we both know how careful people are in reading documentation... > Also pages going in a bio are not necessarily written too, they can > be use as source (writting to block) or as destination (reading from > block). So having all of them with refcount bias as GUP would muddy > the water somemore between pages we can no longer clean (ie GUPed) > and those that are just being use in regular read or write operation. Why would the difference matter here? Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
