* [PATCH] poll/select: initialize triggered field of struct poll_wqueues
@ 2009-08-15 17:30 Guillaume Knispel
2009-08-15 21:41 ` Thomas Gleixner
2009-08-16 0:53 ` Tejun Heo
0 siblings, 2 replies; 7+ messages in thread
From: Guillaume Knispel @ 2009-08-15 17:30 UTC (permalink / raw)
To: linux-kernel
Cc: Tejun Heo, Miklos Szeredi, Andrew Morton, Linus Torvalds,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel
Hi,
That's only the second time I read fs/select.c so this has to be
reviewed carefully.
It has been quickly tested on my Debian Lenny x86_32 box and everything
seems to works fine on the surface.
Cheers,
Guillaume Knispel
poll/select: initialize triggered field of struct poll_wqueues
The triggered field of struct poll_wqueues introduced in commit
5f820f648c92a5ecc771a96b3c29aa6e90013bba "poll: allow f_op->poll to
sleep" was set to 1 in pollwake() (now __pollwake() ), tested and
later set to 0 in poll_schedule_timeout(), but not initialized before.
As a result when the process needs to sleep, triggered was likely to be
non-zero even if pollwake() is not called before the first
poll_schedule_timeout(), meaning schedule_hrtimeout_range() would not
be called and an extra loop calling all ->poll() would be done.
This patch initialize triggered to 0 in poll_initwait() so the ->poll()
are not called twice before the process goes to sleep when it needs to.
Signed-off-by: Guillaume Knispel <gknispel@proformatique.com>
---
fs/select.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index d870237..8084834 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -110,6 +110,7 @@ void poll_initwait(struct poll_wqueues *pwq)
{
init_poll_funcptr(&pwq->pt, __pollwait);
pwq->polling_task = current;
+ pwq->triggered = 0;
pwq->error = 0;
pwq->table = NULL;
pwq->inline_index = 0;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-15 17:30 [PATCH] poll/select: initialize triggered field of struct poll_wqueues Guillaume Knispel
@ 2009-08-15 21:41 ` Thomas Gleixner
2009-08-16 0:53 ` Tejun Heo
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Gleixner @ 2009-08-15 21:41 UTC (permalink / raw)
To: Guillaume Knispel
Cc: linux-kernel, Tejun Heo, Miklos Szeredi, Andrew Morton,
Linus Torvalds, Alexander Viro, Arjan van de Ven, Heiko Carstens,
linux-fsdevel
Guillaume,
On Sat, 15 Aug 2009, Guillaume Knispel wrote:
> poll/select: initialize triggered field of struct poll_wqueues
>
> The triggered field of struct poll_wqueues introduced in commit
> 5f820f648c92a5ecc771a96b3c29aa6e90013bba "poll: allow f_op->poll to
> sleep" was set to 1 in pollwake() (now __pollwake() ), tested and
> later set to 0 in poll_schedule_timeout(), but not initialized before.
> As a result when the process needs to sleep, triggered was likely to be
> non-zero even if pollwake() is not called before the first
> poll_schedule_timeout(), meaning schedule_hrtimeout_range() would not
> be called and an extra loop calling all ->poll() would be done.
>
> This patch initialize triggered to 0 in poll_initwait() so the ->poll()
> are not called twice before the process goes to sleep when it needs to.
Indeed, triggered is random stack content, so it needs to be initialized.
> Signed-off-by: Guillaume Knispel <gknispel@proformatique.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> fs/select.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/select.c b/fs/select.c
> index d870237..8084834 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -110,6 +110,7 @@ void poll_initwait(struct poll_wqueues *pwq)
> {
> init_poll_funcptr(&pwq->pt, __pollwait);
> pwq->polling_task = current;
> + pwq->triggered = 0;
> pwq->error = 0;
> pwq->table = NULL;
> pwq->inline_index = 0;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-15 17:30 [PATCH] poll/select: initialize triggered field of struct poll_wqueues Guillaume Knispel
2009-08-15 21:41 ` Thomas Gleixner
@ 2009-08-16 0:53 ` Tejun Heo
2009-08-16 1:15 ` Andrew Morton
1 sibling, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2009-08-16 0:53 UTC (permalink / raw)
To: Guillaume Knispel
Cc: linux-kernel, Miklos Szeredi, Andrew Morton, Linus Torvalds,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel
Guillaume Knispel wrote:
> poll/select: initialize triggered field of struct poll_wqueues
>
> The triggered field of struct poll_wqueues introduced in commit
> 5f820f648c92a5ecc771a96b3c29aa6e90013bba "poll: allow f_op->poll to
> sleep" was set to 1 in pollwake() (now __pollwake() ), tested and
> later set to 0 in poll_schedule_timeout(), but not initialized before.
> As a result when the process needs to sleep, triggered was likely to be
> non-zero even if pollwake() is not called before the first
> poll_schedule_timeout(), meaning schedule_hrtimeout_range() would not
> be called and an extra loop calling all ->poll() would be done.
>
> This patch initialize triggered to 0 in poll_initwait() so the ->poll()
> are not called twice before the process goes to sleep when it needs to.
>
> Signed-off-by: Guillaume Knispel <gknispel@proformatique.com>
Ah... nice spotting.
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-16 0:53 ` Tejun Heo
@ 2009-08-16 1:15 ` Andrew Morton
2009-08-16 1:37 ` Tejun Heo
2009-08-16 1:39 ` Linus Torvalds
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2009-08-16 1:15 UTC (permalink / raw)
To: Tejun Heo
Cc: Guillaume Knispel, linux-kernel, Miklos Szeredi, Linus Torvalds,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel, stable
On Sun, 16 Aug 2009 09:53:08 +0900 Tejun Heo <tj@kernel.org> wrote:
> Guillaume Knispel wrote:
> > poll/select: initialize triggered field of struct poll_wqueues
> >
> > The triggered field of struct poll_wqueues introduced in commit
> > 5f820f648c92a5ecc771a96b3c29aa6e90013bba "poll: allow f_op->poll to
> > sleep" was set to 1 in pollwake() (now __pollwake() ), tested and
> > later set to 0 in poll_schedule_timeout(), but not initialized before.
> > As a result when the process needs to sleep, triggered was likely to be
> > non-zero even if pollwake() is not called before the first
> > poll_schedule_timeout(), meaning schedule_hrtimeout_range() would not
> > be called and an extra loop calling all ->poll() would be done.
> >
> > This patch initialize triggered to 0 in poll_initwait() so the ->poll()
> > are not called twice before the process goes to sleep when it needs to.
> >
> > Signed-off-by: Guillaume Knispel <gknispel@proformatique.com>
>
> Ah... nice spotting.
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
OK, thanks.
I tagged this for -stable backporting. That isn't strictly needed, as
there's no particular harm caused here. But using an uninitialised
variable in the core poll/select code is pretty gross.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-16 1:15 ` Andrew Morton
@ 2009-08-16 1:37 ` Tejun Heo
2009-08-16 1:39 ` Linus Torvalds
1 sibling, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2009-08-16 1:37 UTC (permalink / raw)
To: Andrew Morton
Cc: Guillaume Knispel, linux-kernel, Miklos Szeredi, Linus Torvalds,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel, stable
Hello,
Andrew Morton wrote:
> OK, thanks.
>
> I tagged this for -stable backporting. That isn't strictly needed, as
> there's no particular harm caused here. But using an uninitialised
> variable in the core poll/select code is pretty gross.
Yes, I was thinking about putting this for -stable too. It's not
critical but still.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-16 1:15 ` Andrew Morton
2009-08-16 1:37 ` Tejun Heo
@ 2009-08-16 1:39 ` Linus Torvalds
2009-08-16 1:40 ` Linus Torvalds
1 sibling, 1 reply; 7+ messages in thread
From: Linus Torvalds @ 2009-08-16 1:39 UTC (permalink / raw)
To: Andrew Morton
Cc: Tejun Heo, Guillaume Knispel, linux-kernel, Miklos Szeredi,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel, stable
On Sat, 15 Aug 2009, Andrew Morton wrote:
>
> I tagged this for -stable backporting. That isn't strictly needed, as
> there's no particular harm caused here. But using an uninitialised
> variable in the core poll/select code is pretty gross.
Hmm. I committed it when Tejun's ack came in, but I didn't tag it for
stable. Oh well.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] poll/select: initialize triggered field of struct poll_wqueues
2009-08-16 1:39 ` Linus Torvalds
@ 2009-08-16 1:40 ` Linus Torvalds
0 siblings, 0 replies; 7+ messages in thread
From: Linus Torvalds @ 2009-08-16 1:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Tejun Heo, Guillaume Knispel, linux-kernel, Miklos Szeredi,
Alexander Viro, Arjan van de Ven, Heiko Carstens,
Thomas Gleixner, linux-fsdevel, stable
On Sat, 15 Aug 2009, Linus Torvalds wrote:
>
> Hmm. I committed it when Tejun's ack came in, but I didn't tag it for
> stable. Oh well.
Oh, actually, I noticed that I hadn't pushed it out yet. So I added the
stable tag and did so.
Linus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-08-16 1:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-15 17:30 [PATCH] poll/select: initialize triggered field of struct poll_wqueues Guillaume Knispel
2009-08-15 21:41 ` Thomas Gleixner
2009-08-16 0:53 ` Tejun Heo
2009-08-16 1:15 ` Andrew Morton
2009-08-16 1:37 ` Tejun Heo
2009-08-16 1:39 ` Linus Torvalds
2009-08-16 1:40 ` Linus Torvalds
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.