On 2022-09-23 17:21, Jason Gunthorpe wrote: > On Fri, Sep 23, 2022 at 05:14:11PM -0600, Logan Gunthorpe wrote: >> >> >> On 2022-09-23 17:07, Jason Gunthorpe wrote: >>> 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.. And on further consideration I really think the correct error return is important here. This will be a user facing error that'll be easy enough to hit: think code that might be run on any file and if the file is hosted on a block device that doesn't support P2PDMA then the user will see the very uninformative "Cannot allocate memory" error. Userspace code that's written for purpose can look at the EREMOTEIO error and tell the user something useful, if we return the correct error. If we return ENOMEM in this case, that is not possible because lots of things might have caused that error. Logan
