>>>> That might be problematic and possibly the wrong approach, depending on >> *what* we're actually pinning and what we're intending to do with that. >> >> My assumption would have been that this interface is to duplicate a pin > > I see that I need to put more documentation here, so people don't have > to assume things... :) > Yes, please :) >> on a page, which would be perfectly fine, because the page actually saw >> a FOLL_PIN previously. >> >> We're taking a pin on a page that we haven't obtained via FOLL_PIN if I >> understand correctly. Which raises the questions, how do we end up with >> the pages here, and what are we doing to do with them (use them like we >> obtained them via FOLL_PIN?)? >> >> >> If it's converting FOLL_GET -> FOLL_PIN manually, then we're bypassing >> FOLL_PIN special handling in GUP code: >> >> page = get_user_pages(FOLL_GET) >> pin_user_page(page) >> put_page(page) > > No, that's not where this is going at all. The idea, which I now see > needs better documentation, is to handle file-backed pages. Only. > > We're not converting from one type to another, nor are we doubling up. > We're just keeping the pin type consistent so that the vast block- > processing machinery can take pages in and handle them, then release > them at the end with bio_release_pages(), which will call > unpin_user_pages(). > Ah, okay, that makes sense. Glad to hear that we're intending to use this with !anon pages only. >> >> >> For anonymous pages, we'll bail out for example once we have >> >> https://lkml.kernel.org/r/20220224122614.94921-14-david@xxxxxxxxxx >> >> Because the conditions for pinned anonymous pages might no longer hold. >> >> If we won't call pin_user_page() on anonymous pages, it would be fine. > > We won't, and in fact, I should add WARN_ON_ONCE(PageAnon(page)) to > this function. Exactly what I would have suggested, > >> But then, I still wonder how we come up the "struct page" here. >> > > From the file system. For example, the NFS-direct and fuse conversions > in the last patches show how that works. > > Thanks for this feedback, this is very helpful. Thanks for clarifying, John! -- Thanks, David / dhildenb
