| From | Linus Torvalds <> | Date | Tue, 4 Jan 2022 13:34:18 -0800 | Subject | Re: [PATCH v6 4/9] mm: multigenerational lru: groundwork |
| |
On Tue, Jan 4, 2022 at 12:23 PM Yu Zhao <yuzhao@google.com> wrote: >
> index a7e4a9e7d807..fadbf8e6abcd 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > > +#ifdef CONFIG_LRU_GEN > +static inline void task_enter_lru_fault(void) > +{ > + WARN_ON_ONCE(current->in_lru_fault); ...
Why are these in this very core header file?
They are used in one single file - mm/memory.c.
They should be just static functions there.
I'm also not sure why the calling convention is
if (lru_fault) task_enter_lru_fault();
instead of doing just
task_enter_lru_fault(vma);
and having that function do
/* Don't do LRU fault accounting for SEQ/RAND files */ if (unlikely(vma->vm_flags & (VM_SEQ_READ | VM_RAND_READ))) return;
which would seem to be a lot more legible and straightforward.
In fact, you could do it without any conditionals at all, if you just remove the WARN_ON_ONCE() from the exit path, turning it into just
current->in_lru_fault = !(vma->vm_flags & (VM_SEQ_READ | VM_RAND_READ));
for 'enter' and just
current->in_lru_fault = 0;
for exit.
It seems pointless to have that extra variable, and the extra conditionals, for a case that is probably very unusual indeed.
Linus
|