Hi Andrea, That all makes sense, thanks so much for that detailed explanation. Brian On Mon, Feb 17, 2020 at 8:27 PM Andrea Arcangeli <aarcange@xxxxxxxxxx> wrote: > > On Mon, Feb 17, 2020 at 07:50:19PM -0600, Brian Geffon wrote: > > But in the meantime, if the plan of record will be to always allow > > retrying then shouldn't the block I mailed a patch on be removed > > regardless because do_user_addr_fault always starts with > > FAULT_FLAG_ALLOW_RETRY and we shouldn't ever land there without it in > > the future and allows userfaultfd to retry? > > It might hide the limitation but only if the page fault originated in > userland (Android's case), but that's not something userfault users > should depend on. Userfaults (unlike sigsegv trapping) are meant to be > reliable and transparent to all user and kernel accesses alike. > > It is also is unclear how long Android will be forced to keep doing > bounce buffers copies in RAM before considering passing any memory to > kernel syscalls. > > For all other users where the kernel access may be the one triggering > the fault the patch will remove a debug aid and the kernel fault would > then fail by hitting on the below: > > /* Not returning to user mode? Handle exceptions or die: */ > no_context(regs, hw_error_code, address, SIGBUS, BUS_ADRERR); > > There may be more side effects in other archs I didn't evaluate > because there's no other place where the common code can return > VM_FAULT_RETRY despite the arch code explicitly told the common code > it can't do that (by not setting FAULT_FLAG_ALLOW_RETRY) so it doesn't > look very safe and it doesn't seem a generic enough solution to the > problem. > > That dump_stack() helped a lot to identify those kernel outliers that > erroneously use get_user_pages instead of the gup_locked/unlocked > variant that are uffd-capable. > > Thanks, > Andrea >
