linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Clark Williams <williams@redhat.com>
Cc: Thomas Gleixner <tglx@glx-um.de>,
	linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	RT <linux-rt-users@vger.kernel.org>,
	Fernando Lopez-Lezcano <nando@ccrma.Stanford.EDU>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: Re: [RFC] mm: change irqs_disabled() test to spin_is_locked() in mem_cgroup_swapout
Date: Fri, 29 May 2015 15:11:59 -0400	[thread overview]
Message-ID: <20150529191159.GA29078@cmpxchg.org> (raw)
In-Reply-To: <20150529104815.2d2e880c@sluggy>

Hi Clark,

On Fri, May 29, 2015 at 10:48:15AM -0500, Clark Williams wrote:
> @@ -5845,7 +5845,7 @@ void mem_cgroup_swapout(struct page *page,
> swp_entry_t entry) page_counter_uncharge(&memcg->memory, 1);
>  
>  	/* XXX: caller holds IRQ-safe mapping->tree_lock */
> -	VM_BUG_ON(!irqs_disabled());
> +	VM_BUG_ON(!spin_is_locked(&page_mapping(page)->tree_lock));
>  
>  	mem_cgroup_charge_statistics(memcg, page, -1);

It's not about the lock, it's about preemption.  The charge statistics
use __this_cpu operations and they're updated from process context and
interrupt context both.

This function really should do a local_irq_save().  I only added the
VM_BUG_ON() to document that we know the caller is holding an IRQ-safe
lock and so we don't need to bother with another level of IRQ saving.

So how does this translate to RT?  I don't know.  But if switching to
explicit IRQ toggling would help you guys out we can do that.  It is
in the swapout path after all, the optimization isn't that important.

  reply	other threads:[~2015-05-29 19:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-29 15:48 [RFC] mm: change irqs_disabled() test to spin_is_locked() in mem_cgroup_swapout Clark Williams
2015-05-29 19:11 ` Johannes Weiner [this message]
2015-05-29 19:21   ` Steven Rostedt
2015-05-29 21:26 ` Andrew Morton
2015-06-01 18:14   ` [RFC][PATCH] mm: ifdef out VM_BUG_ON check on PREEMPT_RT_FULL Clark Williams
2015-06-01 19:00     ` Johannes Weiner
2015-06-01 19:28       ` Steven Rostedt
2015-06-11 11:40       ` Sebastian Andrzej Siewior
2015-06-19 18:00         ` Johannes Weiner
2015-07-08 15:44           ` Sebastian Andrzej Siewior
2015-07-09 15:07             ` Thomas Gleixner
2015-07-09 16:00               ` Johannes Weiner
2015-07-09 16:43                 ` Thomas Gleixner

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=20150529191159.GA29078@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=linux-mm@kvack.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=nando@ccrma.Stanford.EDU \
    --cc=rostedt@goodmis.org \
    --cc=tglx@glx-um.de \
    --cc=williams@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).