On Wed, Mar 9, 2022 at 8:08 PM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Mar 9, 2022 at 10:42 AM Andreas Gruenbacher <agruenba@xxxxxxxxxx> wrote: > > From what I took from the previous discussion, probing at a sub-page > > granularity won't be necessary for bytewise copying: when the address > > we're trying to access is poisoned, fault_in_*() will fail; when we get > > a short result, that will take us to the poisoned address in the next > > iteration. > > Sadly, that isn't actually the case. > > It's not the case for GUP (that page aligns things), and it's not the > case for fault_in_writeable() itself (that also page aligns things). As long as the fault_in_*() functions probe the exact start address, they will detect when that address is poisoned. They can advance page-wise after that and it doesn't matter if they skip poisoned addresses. When the memory range is then accessed, that may fail at a poisoned address. The next call to fault_in_*() will be with that poisoned address as the start address. So it's the unaligned probing at the start in the fault_in_*() functions that's essential, and fault_in_readable(), fault_in_writeable(), and fault_in_safe_writeable() now all do that probing. > But more importantly, it's not actually the case for the *users* > either. Not all of the users are byte-stream oriented, and I think it > was btrfs that had a case of "copy a struct at the beginning of the > stream". And if that copy failed, it wouldn't advance by as many bytes > as it got - it would require that struct to be all fetched, and start > from the beginning. > > So we do need to probe at least a minimum set of bytes. Probably a > fairly small minimum, but still... There are only a few users like that, and they can be changed to pass back the actual address that fails so that that address can be probed instead of probing every 128 bytes of the entire buffer on arm64. Thanks, Andreas
