Re: [PATCH 2/3] mm: workingset: make shadow_lru_isolate() use locking suffix

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2018-06-28 12:30:57 [+0300], Vladimir Davydov wrote:
> > It helps to keep the locking annotation in one place. If it helps I
> > could add the _irqsave() suffix to list_lru_add/del like it is already
> > done in other places (in this file).
> 
> AFAIK local_irqsave/restore don't come for free so using them just to
> keep the code clean doesn't seem to be reasonable.

exactly. So I kept those two as is since there is no need for it.

> > > As for RT, it wouldn't need mm/workingset altogether AFAIU. 
> > Why wouldn't it need it?
> 
> I may be wrong, but AFAIU RT kernel doesn't do swapping.

swapping the RT task out would be bad indeed. This does not stop you
from using it. You can mlock() your RT application (well should because
you don't want do remove RO-data or code from memory because it is
unchanged on disk) and everything else that is not essential (say
SCHED_OTHER) could be swapped out then if memory goes low.

> > invokes. I could also add a different function (say
> > list_lru_walk_one_irq()) which behaves like list_lru_walk_one() but does
> > spin_lock_irq() instead.
> 
> That would look better IMHO. I mean, passing the flag as an argument to
> __list_lru_walk_one and introducing list_lru_shrink_walk_irq.

You think so? So I had this earlier and decided to go with what I
posted. But hey. I will post it later as suggested here and we will see
how it goes.
I just wrote this here to let akpm know that I will do as asked here
(since he Cc: me in other thread on this topic, thank you will act).

Sebastian