archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <>
To: Vlastimil Babka <>
Cc: Axel Rasmussen <>,
	Ingo Molnar <>,
	Andrew Morton <>,
	Michel Lespinasse <>,
	Daniel Jordan <>,
	Jann Horn <>,
	Chinwen Chang <>,
	Davidlohr Bueso <>,
	David Rientjes <>,
	Yafang Shao <>,
	LKML <>,
	Linux MM <>
Subject: Re: [PATCH v4 1/1] mmap_lock: add tracepoints around lock acquisition
Date: Mon, 26 Oct 2020 10:54:28 -0400	[thread overview]
Message-ID: <20201026105428.3205d2b0@gandalf.local.home> (raw)
In-Reply-To: <>

On Fri, 23 Oct 2020 19:56:49 +0200
Vlastimil Babka <> wrote:

> > I'm somewhat sure this code can be called in interrupt context, so I
> > don't think we can use locks to prevent this situation. I think it
> > works like this: say we acquire the lock, an interrupt happens, and
> > then we try to acquire again on the same CPU; we can't sleep, so we're
> > stuck.  
> Yes, we could perhaps trylock() and if it fails, give up on the memcg path.
> > I think we can't kmalloc here (instead of a percpu buffer) either,
> > since I would guess that kmalloc may also acquire mmap_lock itself?  
> the overhead is not worth it anyway, for a tracepoint
> > Is adding local_irq_save()/local_irq_restore() in addition to
> > get_cpu()/put_cpu() sufficient?  
> If you do that, then I guess you don't need get_cpu()/put_cpu() anymore. But 
> it's more costly.
> But sounds like we are solving something that the tracing subystem has to solve 
> as well to store the trace event data, so maybe Steven has some better idea?

How big of a buffer to you need? The way that ftrace handles reserving
buffer for events (which coincidentally, I just talked about today at Open
Source Summit Europe!), is that I simply use local_add_return() and have a
buffer index. And the stack trace code does this as well.

For using a temporary buffer, you would allocate (at enabling of the
tracepoint, which is why we have a "reg" and "unreg" form of the
TRACE_EVENT macro called TRACE_EVENT_FN (for "function)). Have this
temporary buffer per cpu and handle all the contexts that it would be
called in. For ftrace, we usually make it 4 (normal, softirq, irq and NM
context). Ftrace will use local_add_return, but as I'm guessing, the
interrupt context will be done with its buffer after writing the event, you
don't need to worry about the counter being atomic.

You simply need to do:

DEFINE_PER_CPU(char *, my_buffer);
static int my_buf_idx;

At initialization:

	for_each_possible_cpu(cpu) {
		per_cpu(my_buffer, cpu) = kmalloc(MY_BUFF_SIZE *context_needed, GFP_KERNEL);
		if (!per_cpu(my_buffer, cpu))
			goto out_fail;
		per_cpu(my_buf_idx, cpu) = 0;

Then for the event:

	idx = this_cpu_add(my_buf_idx, MY_BUFF_SIZE);
	current_buffer = this_cpu_ptr(my_buffer);
	buf = current_buffer[idx - MY_BUFF-SIZE];


	this_cpu_sub(my_buf_idx, MY_BUFF_SIZE);

Now if an interrupt were to come in, it would do the same thing, but will
use the buffer after the MY_BUFF_SIZE, and you don't need to worry about
one corrupting another. Once the index has been incremented, a interrupt
will use the portion of the buffer after the "allocate" part. And even if
it happened right at the this_cpu_add(), the interrupt would put it back
before returning back to the context that it interrupted.

Is this what you need to deal with?

-- Steve

      reply	other threads:[~2020-10-26 14:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20 18:47 [PATCH v4 0/1] Add tracepoints around mmap_lock acquisition Axel Rasmussen
2020-10-20 18:47 ` [PATCH v4 1/1] mmap_lock: add tracepoints around lock acquisition Axel Rasmussen
2020-10-23 13:59   ` Vlastimil Babka
2020-10-23 17:38     ` Axel Rasmussen
2020-10-23 17:56       ` Vlastimil Babka
2020-10-26 14:54         ` Steven Rostedt [this message]

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:

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

  git send-email \
    --in-reply-to=20201026105428.3205d2b0@gandalf.local.home \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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).