On Wed, Jan 20, 2021 at 9:36 AM Will Deacon <will@xxxxxxxxxx> wrote: > > 'struct vm_fault' contains both information about the fault being > serviced alongside mutable fields contributing to the state of the > fault-handling logic. Unfortunately, the distinction between the two is > not clear-cut, and a number of callers end up manipulating the structure > temporarily before restoring it when returning. > > Try to clean this up by moving the immutable fault information into an > anonymous struct, which will later be marked as 'const'. GCC will then > complain (with an error) about modification of these fields after they > have been initialised, although LLVM currently allows them without even > a warning: > > https://bugs.llvm.org/show_bug.cgi?id=48755 I think this paragraph+link would be better on patch 8/8. > > Ideally, the 'flags' field would be part of the new structure too, but > it seems as though the ->page_mkwrite() path is not ready for this yet. > > Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx> > Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > Link: https://lore.kernel.org/r/CAHk-=whYs9XsO88iqJzN6NC=D-dp2m0oYXuOoZ=eWnvv=5OA+w@xxxxxxxxxxxxxx > Signed-off-by: Will Deacon <will@xxxxxxxxxx> > --- > include/linux/mm.h | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index 251a2339befb..b4a5cb9bff7d 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -517,11 +517,14 @@ static inline bool fault_flag_allow_retry_first(unsigned int flags) > * pgoff should be used in favour of virtual_address, if possible. > */ > struct vm_fault { > - struct vm_area_struct *vma; /* Target VMA */ > - unsigned int flags; /* FAULT_FLAG_xxx flags */ > - gfp_t gfp_mask; /* gfp mask to be used for allocations */ > - pgoff_t pgoff; /* Logical page offset based on vma */ > - unsigned long address; /* Faulting virtual address */ > + struct { > + struct vm_area_struct *vma; /* Target VMA */ > + gfp_t gfp_mask; /* gfp mask to be used for allocations */ > + pgoff_t pgoff; /* Logical page offset based on vma */ > + unsigned long address; /* Faulting virtual address */ > + }; > + unsigned int flags; /* FAULT_FLAG_xxx flags > + * XXX: should really be 'const' */ > pmd_t *pmd; /* Pointer to pmd entry matching > * the 'address' */ > pud_t *pud; /* Pointer to pud entry matching > -- > 2.30.0.284.gd98b1dd5eaa7-goog > -- Thanks, ~Nick Desaulniers
