On Tue, Jan 4, 2022 at 12:23 PM Yu Zhao <yuzhao@xxxxxxxxxx> 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
