On Wed, Jul 10, 2024 at 6:33 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote: > > On 07/03, Oleg Nesterov wrote: > > > > > /* > > > - * The NULL 'tsk' here ensures that any faults that occur here > > > - * will not be accounted to the task. 'mm' *is* current->mm, > > > - * but we treat this as a 'remote' access since it is > > > - * essentially a kernel access to the memory. > > > + * 'mm' *is* current->mm, but we treat this as a 'remote' access since > > > + * it is essentially a kernel access to the memory. > > > */ > > > result = get_user_pages_remote(mm, vaddr, 1, FOLL_FORCE, &page, NULL); > > > > OK, this makes it less confusing, so > > > > Acked-by: Oleg Nesterov <oleg@xxxxxxxxxx> > > > > --------------------------------------------------------------------- > > but it still looks confusing to me. This code used to pass tsk = NULL > > only to avoid tsk->maj/min_flt++ in faultin_page(). > > > > But today mm_account_fault() increments these counters without checking > > FAULT_FLAG_REMOTE, mm == current->mm, so it seems it would be better to > > just use get_user_pages() and remove this comment? > > Well, yes, it still looks confusing, imo. > > Andrii, I hope you won't mind if I redo/resend this and the next cleanup? > > The next one only updates the comment above uprobe_write_opcode(), but > it would be nice to explain mmap_write_lock() in register_for_each_vma(). > I don't mind a bit, thanks for sending the patches! > Oleg. > >
