[PATCH v6 4/9] mm: multigenerational lru: groundwork

Linus Torvalds torvalds at linux-foundation.org
Tue Jan 4 13:34:18 PST 2022


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



More information about the linux-arm-kernel mailing list