On Wed, May 20, 2020 at 03:22:15PM +0300, Konstantin Khlebnikov wrote:
> On 20/05/2020 00.45, Ahmed S. Darwish wrote:
> > Commit eef1a429f234 ("mm/swap.c: piggyback lru_add_drain_all() calls")
> > implemented an optimization mechanism to exit the to-be-started LRU
> > drain operation (name it A) if another drain operation *started and
> > finished* while (A) was blocked on the LRU draining mutex.That commit is horrible... > Well, I thought it fits perfectly =) > > Maybe it's worth to add helpers with appropriate semantics? > This is pretty common pattern. Where's more sites? > > @@ -725,21 +735,48 @@ void lru_add_drain_all(void) > > if (WARN_ON(!mm_percpu_wq)) > > return; > > - seq = raw_read_seqcount_latch(&seqcount); > > mutex_lock(&lock); > > /* > > - * Piggyback on drain started and finished while we waited for lock: > > - * all pages pended at the time of our enter were drained from vectors. > > */ > > - if (__read_seqcount_retry(&seqcount, seq)) > > goto done; Since there is no ordering in raw_read_seqcount_latch(), and mutex_lock() is an ACQUIRE, there's no guarantee the read actually happens before the mutex is acquired. > > - raw_write_seqcount_latch(&seqcount); > > cpumask_clear(&has_work);
