On Tue, Mar 17, 2020 at 02:06:08PM +0100, Christoph Hellwig wrote: > On Tue, Mar 17, 2020 at 09:53:17AM -0300, Jason Gunthorpe wrote: > > > Thinking out loud a bit more: > > > > > > - do we really need HMM_PFN_ERROR, or is just a return value from > > > hmm_range_fault enough? > > > > I'm not totally clear on this. The only use for ERROR is to signal to a > > non-faulting hmm_range_fault (ie shapshot) that the page should generate a > > device fault (ie device SIGSEGV). > > > > We can also handle ERROR by having the device take the fault to CPU, > > then fail during a faulting hmm_range_fault and then dropping the > > ERROR indication toward the device. > > > > If we already know the page cannot be faulted when we read it it feels > > natural to return that too. > > True. Of course we could just stick an ERR_PTR into the page array > in this case. If we make the output here struct page *[] then there is also no reason for hmm_range_fault to exist, get_user_pages already works like that. I see nearly the entire point of hmm_range_fault as being able to return the flags.. > > > - because if it is we don't need output flags at all, and the output > > > array could be struct pages, which would make for a much easier > > > to use API > > > > valid and write is required for the non-faulting case, I don't > > think flags can go away. > > Do we have any cases where these aren't simply kept from the input array? > I couldn't find any, but I've not understood some of the subtle details > before. Not today, I think All this stuff becomes interesting when we have a non-faulting caller of hmm_range_fault(). Then the input request flags will all be 0 and the output flags will indicate the current state of the page table. Jason