public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Prarit Bhargava <prarit@redhat.com>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: linux-kernel@vger.kernel.org, Larry Woodman <lwoodman@redhat.com>,
	Jason Baron <jbaron@redhat.com>, Don Zickus <dzickus@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	john stultz <johnstul@us.ibm.com>
Subject: Re: [RFC PATCH] tty, add kref to sysrq handlers
Date: Fri, 27 Apr 2012 15:46:52 -0400	[thread overview]
Message-ID: <4F9AF7AC.1010708@redhat.com> (raw)
In-Reply-To: <20120427183922.47436912@pyramind.ukuu.org.uk>



On 04/27/2012 01:39 PM, Alan Cox wrote:
>> If the lock were removed, however, there is one _unlikely_ circumstance in
>> which the array would be left unprotected.  That would be the situation in
>> which a module was loaded and the sysrq handler was registered during the
>> execution of the module.  Again, the possibility of the scenario is very
>> small and given the existing usage of unregister_sysrq_key() in the tree
>> it seems like removing the lock is sufficient.
> 
> It's asking for later disasters I think.

Yeah, you're right.

> 
> I'm not sure I see we need a kref - that looks like overkill, and its
> probably even more elegantly done with RCU ?

Possibly -- I'll look into it.

> 
>> Of course, I'm more than willing to hear additional suggestions.  A rw
>> lock still requires that it be taken with irqs disabled so IMO it is out
>> of the question.
> 
> One approach would be to defer the work. Is there any reason a slow sysrq
> handler shouldn't be expected to behave itself and schedule work to run
> later. 

The problem with this is, as lwoodman pointed out me privately, deferring a
sysrq-t is not the best idea.  When sysrq-t is issued we're trying to take a
snapshot of the system as it is at that particular moment, not after some
indeterminate amount of time.  And, if the work scheduler is somehow the problem
(and the reason you're doing a sysrq-t) you won't execute at all.

Thanks for the suggestion Alan -- as always it is much appreciated.  I'll see if
RCU is the solution here.

P.

  reply	other threads:[~2012-04-27 19:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-27 17:15 [RFC PATCH] tty, add kref to sysrq handlers Prarit Bhargava
2012-04-27 17:39 ` Alan Cox
2012-04-27 19:46   ` Prarit Bhargava [this message]
2012-05-04 13:00   ` Prarit Bhargava
  -- strict thread matches above, loose matches on Subject: below --
2012-04-27 17:15 Prarit Bhargava

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=4F9AF7AC.1010708@redhat.com \
    --to=prarit@redhat.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=dzickus@redhat.com \
    --cc=jbaron@redhat.com \
    --cc=johnstul@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lwoodman@redhat.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox