On Fri, Sep 23, 2022 at 05:01:26PM -0600, Logan Gunthorpe wrote: > > > > On 2022-09-23 16:58, Jason Gunthorpe wrote: > > On Fri, Sep 23, 2022 at 02:11:03PM -0600, Logan Gunthorpe wrote: > >> > >> > >> On 2022-09-23 13:53, Jason Gunthorpe wrote: > >>> On Fri, Sep 23, 2022 at 01:08:31PM -0600, Logan Gunthorpe wrote: > >>> I'm encouraging Dan to work on better infrastructure in pgmap core > >>> because every pgmap implementation has this issue currently. > >>> > >>> For that reason it is probably not so relavent to this series. > >>> > >>> Perhaps just clarify in the commit message that the FOLL_LONGTERM > >>> restriction is to copy DAX until the pgmap page refcounts are fixed. > >> > >> Ok, I'll add that note. > >> > >> Per the fix for the try_grab_page(), to me it doesn't fit well in > >> try_grab_page() without doing a bunch of cleanup to change the > >> error handling, and the same would have to be added to try_grab_folio(). > >> So I think it's better to leave it where it was, but move it below the > >> respective grab calls. Does the incremental patch below look correct? > > > > Oh? I was thinking of just a very simple thing: > > Really would like it to return -EREMOTEIO instead of -ENOMEM as that's the > error used for bad P2PDMA page everywhere. I'd rather not see GUP made more fragile just for that.. > Plus the concern that some of the callsites of try_grab_page() might not have > a get or a pin and thus it's not safe which was the whole point of the change > anyway. try_grab_page() calls folio_ref_inc(), that is only legal if it knows the page is already a valid pointer under the PTLs, so it is safe to check the pgmap as well. Jason
