All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: Tejun Heo <tj@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: try_to_freeze() called with IRQs disabled on ARM
Date: Thu, 25 Aug 2011 14:04:14 +0100	[thread overview]
Message-ID: <20110825130414.GE8883@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110825123542.GM3286@htj.dyndns.org>

On Thu, Aug 25, 2011 at 02:35:42PM +0200, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 25, 2011 at 01:25:43PM +0100, Russell King - ARM Linux wrote:
> > No.  Stop bodging and hiding problems.  Anywhere which does this:
> > 
> > 	local_irq_enable();
> > 	do something
> > 	local_irq_disable();
> > 
> > is a bug.  Things are called with IRQs disabled for a reason - randomly
> > re-enabling IRQs does not "fix" stuff, it merely introduces subtle bugs
> > while hiding warnings of those bugs.
> > 
> > Please go back and read my response to Mark at the beginning of this
> > thread, where I describe why IRQs are disabled here.
> > 
> > The only solution here is to fix the problem properly, and I'm working
> > on a patch to fix the problem I highlighted in my earlier response to
> > Mark.  Once we have that problem fixed, we can then (more) safely call
> > do_signal() with IRQs enabled.
> 
> Arrrrrrrrgghhhhhhhhhhhh, why is communication so difficult with you?
> When did I ever suggest that as a proper fix.

I'm not the problem here - I'm pointing out that your suggestion is just
going to make things worse not better.  But it seems you can't cope with
people who criticise your solutions in any way.

> ARM is frigging broken
> in that path so don't push it over to PM and just deal with it inside
> ARM arch code.

For fuck sake, who said anything about pushing it over to PM to deal
with?

I'm talking about not putting sticky plasters over the problem, but fixing
the problem PROPERLY.  I'm sorry if doing a job properly offends you, but
you have to live with that.

I _HATE_ solutions which just paper over problems and make then disappear.
I've always been firmly in the "if there's a problem, fix the problem
properly" camp.  You seem to be firmly in the "lets paper over the problem
and forget about it".

> For now, we need to make the warning go away until it's properly fixed
> so turn off and on IRQ around get_signal_to_deliver() and mark it with
> giant FIXME comment.

No.  This is where we disagree.  It doesn't warrant that because it is
possible to fix the problem properly.  While _you_ may not be capable of
doing that, that's no reason not to seek help to have it fixed.

Given that I've posted a description of the problem, would it not be
reasonable to assume that I'm working on fixing it?  Strangely enough
before writing _any_ of the replies to this thread, I have a patch which
does just that - I just haven't tested it yet apart from a compile test.

> How many times did I write that?  I don't know how to make that any
> clearer to you.  Gees...

Hahaha.  Thanks for the laugh - because you never actually said that -
and you're now blaming me for miscommunication?

WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: try_to_freeze() called with IRQs disabled on ARM
Date: Thu, 25 Aug 2011 14:04:14 +0100	[thread overview]
Message-ID: <20110825130414.GE8883@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <20110825123542.GM3286@htj.dyndns.org>

On Thu, Aug 25, 2011 at 02:35:42PM +0200, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 25, 2011 at 01:25:43PM +0100, Russell King - ARM Linux wrote:
> > No.  Stop bodging and hiding problems.  Anywhere which does this:
> > 
> > 	local_irq_enable();
> > 	do something
> > 	local_irq_disable();
> > 
> > is a bug.  Things are called with IRQs disabled for a reason - randomly
> > re-enabling IRQs does not "fix" stuff, it merely introduces subtle bugs
> > while hiding warnings of those bugs.
> > 
> > Please go back and read my response to Mark at the beginning of this
> > thread, where I describe why IRQs are disabled here.
> > 
> > The only solution here is to fix the problem properly, and I'm working
> > on a patch to fix the problem I highlighted in my earlier response to
> > Mark.  Once we have that problem fixed, we can then (more) safely call
> > do_signal() with IRQs enabled.
> 
> Arrrrrrrrgghhhhhhhhhhhh, why is communication so difficult with you?
> When did I ever suggest that as a proper fix.

I'm not the problem here - I'm pointing out that your suggestion is just
going to make things worse not better.  But it seems you can't cope with
people who criticise your solutions in any way.

> ARM is frigging broken
> in that path so don't push it over to PM and just deal with it inside
> ARM arch code.

For fuck sake, who said anything about pushing it over to PM to deal
with?

I'm talking about not putting sticky plasters over the problem, but fixing
the problem PROPERLY.  I'm sorry if doing a job properly offends you, but
you have to live with that.

I _HATE_ solutions which just paper over problems and make then disappear.
I've always been firmly in the "if there's a problem, fix the problem
properly" camp.  You seem to be firmly in the "lets paper over the problem
and forget about it".

> For now, we need to make the warning go away until it's properly fixed
> so turn off and on IRQ around get_signal_to_deliver() and mark it with
> giant FIXME comment.

No.  This is where we disagree.  It doesn't warrant that because it is
possible to fix the problem properly.  While _you_ may not be capable of
doing that, that's no reason not to seek help to have it fixed.

Given that I've posted a description of the problem, would it not be
reasonable to assume that I'm working on fixing it?  Strangely enough
before writing _any_ of the replies to this thread, I have a patch which
does just that - I just haven't tested it yet apart from a compile test.

> How many times did I write that?  I don't know how to make that any
> clearer to you.  Gees...

Hahaha.  Thanks for the laugh - because you never actually said that -
and you're now blaming me for miscommunication?

  reply	other threads:[~2011-08-25 13:04 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-23 15:19 try_to_freeze() called with IRQs disabled on ARM Mark Brown
2011-08-23 15:19 ` Mark Brown
2011-08-23 15:43 ` Russell King - ARM Linux
2011-08-23 15:43   ` Russell King - ARM Linux
2011-08-23 22:08   ` Rafael J. Wysocki
2011-08-23 22:08     ` Rafael J. Wysocki
2011-08-23 21:51 ` Rafael J. Wysocki
2011-08-23 21:51   ` Rafael J. Wysocki
2011-08-23 21:53   ` Tejun Heo
2011-08-23 21:53     ` Tejun Heo
2011-08-23 22:00     ` Russell King - ARM Linux
2011-08-23 22:00       ` Russell King - ARM Linux
2011-08-23 22:08       ` Tejun Heo
2011-08-23 22:08         ` Tejun Heo
2011-08-23 22:13         ` Russell King - ARM Linux
2011-08-23 22:13           ` Russell King - ARM Linux
2011-08-23 22:17           ` Tejun Heo
2011-08-23 22:17             ` Tejun Heo
2011-08-23 22:35             ` Tejun Heo
2011-08-23 22:35               ` Tejun Heo
2011-08-24 23:15               ` Rafael J. Wysocki
2011-08-24 23:15                 ` Rafael J. Wysocki
2011-08-25 12:14             ` Russell King - ARM Linux
2011-08-25 12:14               ` Russell King - ARM Linux
2011-08-25 12:17               ` Tejun Heo
2011-08-25 12:17                 ` Tejun Heo
2011-08-25 12:25                 ` Russell King - ARM Linux
2011-08-25 12:25                   ` Russell King - ARM Linux
2011-08-25 12:35                   ` Tejun Heo
2011-08-25 12:35                     ` Tejun Heo
2011-08-25 13:04                     ` Russell King - ARM Linux [this message]
2011-08-25 13:04                       ` Russell King - ARM Linux
2011-08-25 13:09                       ` Tejun Heo
2011-08-25 13:09                         ` Tejun Heo
2011-08-25 14:55                         ` Russell King - ARM Linux
2011-08-25 14:55                           ` Russell King - ARM Linux
2011-08-26 14:44                           ` Arnd Bergmann
2011-08-26 14:44                             ` Arnd Bergmann
2011-09-01 13:41                             ` Ulrich Weigand
2011-09-01 13:41                               ` Ulrich Weigand
2011-09-01 14:00                               ` Russell King - ARM Linux
2011-09-01 14:00                                 ` Russell King - ARM Linux
2011-09-02 14:47                                 ` Ulrich Weigand
2011-09-02 14:47                                   ` Ulrich Weigand
2011-09-02 17:22                                   ` Russell King - ARM Linux
2011-09-02 17:22                                     ` Russell King - ARM Linux
2011-09-02 17:40                                     ` Ulrich Weigand
2011-09-02 17:40                                       ` Ulrich Weigand
2011-09-02 17:48                                       ` Russell King - ARM Linux
2011-09-02 17:48                                         ` Russell King - ARM Linux
2011-09-16 10:31                                         ` Martin Schwidefsky
2011-09-16 10:31                                           ` Martin Schwidefsky
2011-09-27 17:45                                         ` Ulrich Weigand
2011-09-27 17:45                                           ` Ulrich Weigand
2011-08-30 20:58                           ` Mark Brown
2011-08-30 20:58                             ` Mark Brown
2011-08-30 21:10                             ` Russell King - ARM Linux
2011-08-30 21:10                               ` Russell King - ARM Linux
2012-06-26 16:39                           ` Mandeep Singh Baines
2012-06-26 16:39                             ` Mandeep Singh Baines
2012-06-26 17:16                             ` Russell King - ARM Linux
2012-06-26 17:16                               ` Russell King - ARM Linux
2011-08-23 22:13     ` Rafael J. Wysocki
2011-08-23 22:13       ` Rafael J. Wysocki
2011-08-25 11:37   ` Mark Brown
2011-08-25 11:37     ` Mark Brown

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=20110825130414.GE8883@n2100.arm.linux.org.uk \
    --to=linux@arm.linux.org.uk \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=tj@kernel.org \
    /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.