From: Martin Pieuchot Subject: Re: pgo_fault & VM_PAGER_* values To: Mark Kettenis Cc: tb@openbsd.org, tech@openbsd.org, jsg@openbsd.org Date: Mon, 9 Dec 2024 12:28:01 +0100 On 09/12/24(Mon) 12:21, Mark Kettenis wrote: > > Date: Mon, 9 Dec 2024 11:38:53 +0100 > > From: Martin Pieuchot > > Not looked at the complete diff in detail yet, but I spotted something > that raised a question about the approach taken: > > You're mapping both VM_PAGER_BAD and VM_PAGER_ERROR to EACCESS here. > But those really are different things and the difference should really > be visible in userland. When I wrote this code, my expectation was > that return VM_PAGER_ERROR would result in uvm_fault() returning EIO, > which would then be translated into a SIGBUS. Now, looking at > uvm_fault.c it seems we currently do return EACCESS in that case. I'm not doing it. It is already like that. I agree with all you said and I also want more EIO to be returned. > Maybe that was always wrong or maybe this got lost in some kind of > refactoring. That was always wrong and that's why I added some XXX below where we could change EACCESS into EIO. More could be converted afterward, I'm not interested in doing this work for the moment. > Now you probably don't want to introduce a change of behaviour with > this diff. But maybe it is a good idea to mark those VM_PAGER_ERROR > cases that now become EACCESS with /* XXX */ such that we can revisit > them later. That's what you did with the -ENOMEM case in > i915_error_to_vmf_fault(), which we may indeed want to propagate up as > ENOMEM instead of EACCESS as well. But that needs a bit of thought. That's exactly what I did. > Also, returning errno values instead of the VM_PAGER_XXX constants > means that it becomes less obvious what error values can actually be > returned. We probably should document the accepted error values > somewhere. Maybe we should add those to the uvm_fault(9) page? Sure, I'll add that to my VM_PAGER_* removal todo list. > Cheers, Thanks for the review. Martin