All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxim Levitsky <maximlevitsky@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: akpm <akpm@linux-foundation.org>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	linuxppc-dev list <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] synchronize_irq needs a barrier
Date: Sat, 20 Oct 2007 05:10:01 +0200	[thread overview]
Message-ID: <200710200510.01409.maximlevitsky@gmail.com> (raw)
In-Reply-To: <alpine.LFD.0.999.0710191924520.3794@woody.linux-foundation.org>

On Saturday 20 October 2007 04:25:34 Linus Torvalds wrote:
> 
> On Sat, 20 Oct 2007, Maxim Levitsky wrote:
> > 
> > and the interrupt handler:
> > 
> > 	smp_rmb();
> > 	if (dev->insuspend)
> > 		goto out;
> 
> Something like that can work, yes. However, you need to make sure that:
> 
>  - even when you ignore the interrupt (because the driver doesn't care, 
>    it's suspending), you need to make sure the hardware gets shut up by 
>    reading (or writing) the proper interrupt status register.
I agree, but while device is powered off, its registers can't be accessed
Thus, if I ack the IRQ every time the handler is called, I will access the 
powered off device (this is probably won't hurt a lot, but a bit incorrectly)

> 
>    Otherwise, with a level interrupt, the interrupt will continue to be 
>    held active ("screaming") and the CPU will get interrupted over and 
>    over again, until the irq subsystem will just turn the irq off 
>    entirely.
> 
>  - when you resume, make sure that you get the engine going again, with 
>    the understanding that some interrupts may have gotten ignored.
Here it isn't a problem, this is a video capture card, and I suspend it by just stopping dma
on all active buffers even if in the middle of capture, and I send those buffers to card again
to fill them from the beginning during the resume.
> 
> Also, in general, these kinds of things shouldn't always even be 
> neicessary. If you use the suspend_late()/resume_early() hooks, those will 
> be called with interrupts off, and while they can be harder to debug (and 
> may be worth avoiding for non-critical drivers), they do allow for simpler 
> models partly exactly because they don't need to worry about interrupts 
> etc.
Exactly, I am aware of suspend_late , but I don't want to abuse it.
> 
> 		Linus
> 


Best regards,
	Maxim Levitsky

WARNING: multiple messages have this Message-ID (diff)
From: Maxim Levitsky <maximlevitsky@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: benh@kernel.crashing.org, akpm <akpm@linux-foundation.org>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	linuxppc-dev list <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] synchronize_irq needs a barrier
Date: Sat, 20 Oct 2007 05:10:01 +0200	[thread overview]
Message-ID: <200710200510.01409.maximlevitsky@gmail.com> (raw)
In-Reply-To: <alpine.LFD.0.999.0710191924520.3794@woody.linux-foundation.org>

On Saturday 20 October 2007 04:25:34 Linus Torvalds wrote:
> 
> On Sat, 20 Oct 2007, Maxim Levitsky wrote:
> > 
> > and the interrupt handler:
> > 
> > 	smp_rmb();
> > 	if (dev->insuspend)
> > 		goto out;
> 
> Something like that can work, yes. However, you need to make sure that:
> 
>  - even when you ignore the interrupt (because the driver doesn't care, 
>    it's suspending), you need to make sure the hardware gets shut up by 
>    reading (or writing) the proper interrupt status register.
I agree, but while device is powered off, its registers can't be accessed
Thus, if I ack the IRQ every time the handler is called, I will access the 
powered off device (this is probably won't hurt a lot, but a bit incorrectly)

> 
>    Otherwise, with a level interrupt, the interrupt will continue to be 
>    held active ("screaming") and the CPU will get interrupted over and 
>    over again, until the irq subsystem will just turn the irq off 
>    entirely.
> 
>  - when you resume, make sure that you get the engine going again, with 
>    the understanding that some interrupts may have gotten ignored.
Here it isn't a problem, this is a video capture card, and I suspend it by just stopping dma
on all active buffers even if in the middle of capture, and I send those buffers to card again
to fill them from the beginning during the resume.
> 
> Also, in general, these kinds of things shouldn't always even be 
> neicessary. If you use the suspend_late()/resume_early() hooks, those will 
> be called with interrupts off, and while they can be harder to debug (and 
> may be worth avoiding for non-critical drivers), they do allow for simpler 
> models partly exactly because they don't need to worry about interrupts 
> etc.
Exactly, I am aware of suspend_late , but I don't want to abuse it.
> 
> 		Linus
> 


Best regards,
	Maxim Levitsky


  reply	other threads:[~2007-10-20  3:10 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-10-18  1:25 [PATCH] synchronize_irq needs a barrier Benjamin Herrenschmidt
2007-10-18  1:25 ` Benjamin Herrenschmidt
2007-10-18  1:45 ` Andrew Morton
2007-10-18  1:45   ` Andrew Morton
2007-10-18  1:55   ` Benjamin Herrenschmidt
2007-10-18  1:55     ` Benjamin Herrenschmidt
2007-10-18  2:12 ` Linus Torvalds
2007-10-18  2:12   ` Linus Torvalds
2007-10-18  2:40   ` Benjamin Herrenschmidt
2007-10-18  2:40     ` Benjamin Herrenschmidt
2007-10-18  2:57     ` Benjamin Herrenschmidt
2007-10-18  2:57       ` Benjamin Herrenschmidt
2007-10-18 14:56       ` Herbert Xu
2007-10-18 14:56         ` Herbert Xu
2007-10-18 22:05         ` Benjamin Herrenschmidt
2007-10-18 22:05           ` Benjamin Herrenschmidt
2007-10-18 22:52           ` Linus Torvalds
2007-10-18 22:52             ` Linus Torvalds
2007-10-18 23:17             ` Benjamin Herrenschmidt
2007-10-18 23:17               ` Benjamin Herrenschmidt
2007-10-18 23:39               ` Linus Torvalds
2007-10-18 23:39                 ` Linus Torvalds
2007-10-18 23:52                 ` Benjamin Herrenschmidt
2007-10-18 23:52                   ` Benjamin Herrenschmidt
2007-10-19  2:32                 ` Herbert Xu
2007-10-19  2:32                   ` Herbert Xu
2007-10-19  2:52                   ` Nick Piggin
2007-10-19  2:52                     ` Nick Piggin
2007-10-19  3:28                     ` Herbert Xu
2007-10-19  3:28                       ` Herbert Xu
2007-10-19  4:49                       ` Nick Piggin
2007-10-19  4:49                         ` Nick Piggin
2007-10-19  2:55                   ` Linus Torvalds
2007-10-19  2:55                     ` Linus Torvalds
2007-10-19  3:26                     ` Linus Torvalds
2007-10-19  3:26                       ` Linus Torvalds
2007-10-19  4:11                       ` Benjamin Herrenschmidt
2007-10-19  4:11                         ` Benjamin Herrenschmidt
2007-10-19  4:26                         ` Benjamin Herrenschmidt
2007-10-19  4:26                           ` Benjamin Herrenschmidt
2007-10-19  5:53                           ` Herbert Xu
2007-10-19  5:53                             ` Herbert Xu
2007-10-19  4:20                       ` Herbert Xu
2007-10-19  4:20                         ` Herbert Xu
2007-10-19  4:29                         ` Benjamin Herrenschmidt
2007-10-19  4:29                           ` Benjamin Herrenschmidt
2007-10-19  4:35                         ` Benjamin Herrenschmidt
2007-10-19  4:35                           ` Benjamin Herrenschmidt
2007-10-19  4:48                         ` Herbert Xu
2007-10-19  4:48                           ` Herbert Xu
2007-10-19  4:58                           ` Benjamin Herrenschmidt
2007-10-19  4:58                             ` Benjamin Herrenschmidt
2007-10-21 21:10                             ` Benjamin Herrenschmidt
2007-10-21 21:10                               ` Benjamin Herrenschmidt
2007-10-23  3:26                               ` [IRQ]: Fix synchronize_irq races with IRQ handler Herbert Xu
2007-10-23  3:26                                 ` Herbert Xu
2007-10-19  5:36                         ` [NET]: Fix possible dev_deactivate race condition Herbert Xu
2007-10-19  5:36                           ` Herbert Xu
2007-10-19  5:38                           ` David Miller
2007-10-19  5:38                             ` David Miller
2007-10-19  7:35                           ` Peter Zijlstra
2007-10-19  7:35                             ` Peter Zijlstra
2007-10-19  9:29                             ` Herbert Xu
2007-10-19  9:29                               ` Herbert Xu
2007-10-18 14:35     ` [PATCH] synchronize_irq needs a barrier Herbert Xu
2007-10-18 14:35       ` Herbert Xu
2007-10-18 21:35       ` Benjamin Herrenschmidt
2007-10-18 21:35         ` Benjamin Herrenschmidt
2007-10-20  2:02 ` Maxim Levitsky
2007-10-20  2:02   ` Maxim Levitsky
2007-10-20  2:25   ` Linus Torvalds
2007-10-20  2:25     ` Linus Torvalds
2007-10-20  3:10     ` Maxim Levitsky [this message]
2007-10-20  3:10       ` Maxim Levitsky
2007-10-20  4:06       ` Benjamin Herrenschmidt
2007-10-20  4:06         ` Benjamin Herrenschmidt
2007-10-20  4:04     ` Benjamin Herrenschmidt
2007-10-20  4:04       ` Benjamin Herrenschmidt
2007-10-20  4:09       ` Benjamin Herrenschmidt
2007-10-20  4:09         ` Benjamin Herrenschmidt
2007-10-20  3:37   ` Herbert Xu
2007-10-20  3:37     ` Herbert Xu
2007-10-20  3:56   ` Benjamin Herrenschmidt
2007-10-20  3:56     ` Benjamin Herrenschmidt
2007-10-20  4:24     ` Maxim Levitsky
2007-10-20  4:24       ` Maxim Levitsky
2007-10-20  5:04       ` Benjamin Herrenschmidt
2007-10-20  5:04         ` Benjamin Herrenschmidt
2007-10-20  5:36         ` Maxim Levitsky
2007-10-20  5:36           ` Maxim Levitsky
2007-10-20  5:46           ` Benjamin Herrenschmidt
2007-10-20  5:46             ` Benjamin Herrenschmidt
2007-10-20  6:06             ` Maxim Levitsky
2007-10-20  6:06               ` Maxim Levitsky
2007-10-20  6:13               ` Benjamin Herrenschmidt
2007-10-20  6:13                 ` Benjamin Herrenschmidt

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=200710200510.01409.maximlevitsky@gmail.com \
    --to=maximlevitsky@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=torvalds@linux-foundation.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.