mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>,
	Vaibhav Nagarnaik <vnagarnaik@google.com>,
	Ingo Molnar <mingo@redhat.com>, Michael Rubin <mrubin@google.com>,
	David Sharp <dhsharp@google.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] trace: Check for waiting readers before calling wakeup()
Date: Thu, 05 May 2011 08:39:06 -0400	[thread overview]
Message-ID: <1304599146.25414.2484.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <BANLkTimMvEyzG-S=S6R4fVsui-9ok63jwg@mail.gmail.com>

On Wed, 2011-05-04 at 20:53 -0700, Linus Torvalds wrote:

> I dunno. It's sometimes safe, but it's equally often not safe unless
> there is some other locking around the thing. So I'd be a bit nervous.
> The tracer people would need to do some serious thinking about it.

The problem with tracing is that it can be very intrusive. We want the
data but we also do not want to affect the system either. We are
constantly fighting with Heissenberg on this.

Perhaps we could do what perf currently does and use irq_work() instead.
This should be lighter weight. The wakeup should not be at the tracing
event itself as the wakeup is too invasive.

Maybe we could use bitmasks in this case, but I would also hate to use
memory barriers here, although a memory barrier might be much lighter
weight than spinlocks that we currently are fighting with.

If irq_work() doesn't work, perhaps just have something like:

event() {
	write_data();
	smp_mb();
	if (test_and_clear_bit())
		wakeup();
}

reader() {
	set_bit();	
	smp_mb();
	if (data_empty())
		schedule();
	clear_bit();
	smp_mb();
	wake_up_other_readers_if_needed()
}

This is probably better than a spinlock.

-- Steve



  parent reply	other threads:[~2011-05-05 12:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-04  0:05 Vaibhav Nagarnaik
2011-05-04 21:21 ` Vaibhav Nagarnaik
2011-05-04 21:26   ` Frederic Weisbecker
2011-05-05  3:53     ` Linus Torvalds
2011-05-05  6:05       ` Vaibhav Nagarnaik
2011-05-05 12:39       ` Steven Rostedt [this message]
2011-05-10 20:27 ` [PATCH v2] trace: Schedule a delayed work to call wakeup() Vaibhav Nagarnaik
2011-05-17 18:01   ` Steven Rostedt
2011-05-17 20:06     ` Vaibhav Nagarnaik
2011-07-05 12:48   ` [tip:perf/core] tracing: " tip-bot for Vaibhav Nagarnaik

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=1304599146.25414.2484.camel@gandalf.stny.rr.com \
    --to=rostedt@goodmis.org \
    --cc=dhsharp@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mrubin@google.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vnagarnaik@google.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

all inboxes | Powered by JetHome®