All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: linux-mm@kvack.org, Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2] mm: workingset: replace IRQ-off check with a lockdep assert.
Date: Wed, 13 Feb 2019 10:27:54 +0100	[thread overview]
Message-ID: <20190213092754.baxi5zpe7kdpf3bj@linutronix.de> (raw)
In-Reply-To: <20190211210208.GA9580@cmpxchg.org>

On 2019-02-11 16:02:08 [-0500], Johannes Weiner wrote:
> > how do you define safe? I've been looking for dependencies of
> > __mod_lruvec_state() but found only that the lock is held during the RMW
> > operation with WORKINGSET_NODES idx.
> 
> These stat functions are not allowed to nest, and the executing thread
> cannot migrate to another CPU during the operation, otherwise they
> corrupt the state they're modifying.

If everyone is taking the same lock (like i_pages.xa_lock) then there
will not be two instances updating the same stat. The owner of the
(sleeping)-spinlock will not be migrated to another CPU.

> They are called from interrupt handlers, such as when NR_WRITEBACK is
> decreased. Thus workingset_node_update() must exclude preemption from
> irq handlers on the local CPU.

Do you have an example for a code path to check NR_WRITEBACK?
 
> They rely on IRQ-disabling to also disable CPU migration.
The spinlock disables CPU migration. 

> > >                                            I'm guessing it's because
> > > preemption is disabled and irq handlers are punted to process context.
> > preemption is enabled and IRQ are processed in forced-threaded mode.
> 
> That doesn't sound safe.

Do you have test-case or something I could throw at it and verify that
this still works? So far nothing complains…

Sebastian


  reply	other threads:[~2019-02-13  9:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-11  9:57 [PATCH] mm: workingset: replace IRQ-off check with a lockdep assert Sebastian Andrzej Siewior
2019-02-11 11:38 ` [PATCH v2] " Sebastian Andrzej Siewior
2019-02-11 18:53   ` Johannes Weiner
2019-02-11 19:13     ` Sebastian Andrzej Siewior
2019-02-11 19:17       ` Matthew Wilcox
2019-02-11 19:41         ` Sebastian Andrzej Siewior
2019-02-11 21:02       ` Johannes Weiner
2019-02-13  9:27         ` Sebastian Andrzej Siewior [this message]
2019-02-13 14:56           ` Johannes Weiner
2019-08-21 11:21             ` Sebastian Andrzej Siewior
2019-08-21 15:21               ` Johannes Weiner
2019-02-11 17:07 ` [PATCH] " kbuild test robot
2019-02-11 17:37 ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190213092754.baxi5zpe7kdpf3bj@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=linux-mm@kvack.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.