Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Tue, 2016-05-03 at 15:40 +0200, Oleg Nesterov wrote: > On 05/02, Andrew Morton wrote: > > > > > > On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenko wrote: > > > > > > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). > We have TASK_NOLOAD/TASK_IDLE, you can just use schedule_timeout_idle(HZ). [...] I wasn't aware of that function... ah, that would be because it's new in 4.6. It seems much clearer to use that than to use msleep_interruptible() and ignore the result. Ben. -- Ben Hutchings In a hierarchy, every employee tends to rise to his level of incompetence. signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Tue, 2016-05-03 at 15:40 +0200, Oleg Nesterov wrote: > On 05/02, Andrew Morton wrote: > > > > > > On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenko wrote: > > > > > > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). > We have TASK_NOLOAD/TASK_IDLE, you can just use schedule_timeout_idle(HZ). [...] I wasn't aware of that function... ah, that would be because it's new in 4.6. It seems much clearer to use that than to use msleep_interruptible() and ignore the result. Ben. -- Ben Hutchings In a hierarchy, every employee tends to rise to his level of incompetence. signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On 05/02, Andrew Morton wrote: > > On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenko >wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). We have TASK_NOLOAD/TASK_IDLE, you can just use schedule_timeout_idle(HZ). but msleep_interruptible(1000) is fine too. > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > if (host->eject) > > break; > > > > - msleep(1000); > > + if (msleep_interruptible(1000)) > > + flush_signals(current); > > } > > > > complete(>detect_ms_exit); > > flush_signals() is a bit scary. ... > But this isn't a userspace task - it's a kthread. So I don't *think* > it can get any signals anyway? Agreed, it is not needed and only adds some confusion, so I think rtsx_usb_ms-use-msleep_interruptible-in-polling-loop.patch should be updated. A kernel thread ignores all signals unless it does allow_signal(), so you can safely remove flush_signals(). Oleg.
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On 05/02, Andrew Morton wrote: > > On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenko > wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). We have TASK_NOLOAD/TASK_IDLE, you can just use schedule_timeout_idle(HZ). but msleep_interruptible(1000) is fine too. > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > if (host->eject) > > break; > > > > - msleep(1000); > > + if (msleep_interruptible(1000)) > > + flush_signals(current); > > } > > > > complete(>detect_ms_exit); > > flush_signals() is a bit scary. ... > But this isn't a userspace task - it's a kthread. So I don't *think* > it can get any signals anyway? Agreed, it is not needed and only adds some confusion, so I think rtsx_usb_ms-use-msleep_interruptible-in-polling-loop.patch should be updated. A kernel thread ignores all signals unless it does allow_signal(), so you can safely remove flush_signals(). Oleg.
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenkowrote: > This patch has already been posted to LKML by Ben Hutchings ~6 months > ago, but AFAIK no further action were performed. However, this patch > really fixes weird loadavg with RTS5129 card reader, so I would wonder > if this could be merged. AFAIK, it has been applied to some distros' > kernels, e.g., Ubuntu. > > Original Ben's message goes below. > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > hm. > index 1105db2..645dede 100644 > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); flush_signals() is a bit scary. If this was a userspace task and it had (say) SIGINT pending then it would be very rude for a device driver to rub that out. But this isn't a userspace task - it's a kthread. So I don't *think* it can get any signals anyway? And looking at Oleg's 9e7c8f8c62c1e1cda203b it appears that flush_signals() is for flushing signals of a userspace task, not a kthread? Despite the comment "Flush all pending signals for this kthread". Confused. It's been a while since I looked at this stuff and people have mucked with it. Oleg, can you please sort me out?
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 02 May 2016 23:17:41 +0300 Oleksandr Natalenko wrote: > This patch has already been posted to LKML by Ben Hutchings ~6 months > ago, but AFAIK no further action were performed. However, this patch > really fixes weird loadavg with RTS5129 card reader, so I would wonder > if this could be merged. AFAIK, it has been applied to some distros' > kernels, e.g., Ubuntu. > > Original Ben's message goes below. > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > hm. > index 1105db2..645dede 100644 > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); flush_signals() is a bit scary. If this was a userspace task and it had (say) SIGINT pending then it would be very rude for a device driver to rub that out. But this isn't a userspace task - it's a kthread. So I don't *think* it can get any signals anyway? And looking at Oleg's 9e7c8f8c62c1e1cda203b it appears that flush_signals() is for flushing signals of a userspace task, not a kthread? Despite the comment "Flush all pending signals for this kthread". Confused. It's been a while since I looked at this stuff and people have mucked with it. Oleg, can you please sort me out?
[PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
This patch has already been posted to LKML by Ben Hutchings ~6 months ago, but AFAIK no further action were performed. However, this patch really fixes weird loadavg with RTS5129 card reader, so I would wonder if this could be merged. AFAIK, it has been applied to some distros' kernels, e.g., Ubuntu. Original Ben's message goes below. rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben HutchingsSigned-off-by: Oleksandr Natalenko --- drivers/memstick/host/rtsx_usb_ms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c index 1105db2..645dede 100644 --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); -- 2.8.2
[PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
This patch has already been posted to LKML by Ben Hutchings ~6 months ago, but AFAIK no further action were performed. However, this patch really fixes weird loadavg with RTS5129 card reader, so I would wonder if this could be merged. AFAIK, it has been applied to some distros' kernels, e.g., Ubuntu. Original Ben's message goes below. rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben Hutchings Signed-off-by: Oleksandr Natalenko --- drivers/memstick/host/rtsx_usb_ms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c index 1105db2..645dede 100644 --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); -- 2.8.2
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Thu, 08 Oct 2015, Ben Hutchings wrote: > On Thu, 2015-10-08 at 08:19 +0100, Lee Jones wrote: > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). A load average of ~1 on a system that > > > should be idle is somewhat alarming. > > > > > > Change the sleep to be interruptible, but still ignore signals. > > > > > > A better fix might be to replace this loop with a delayed work item. > > > > > > References: https://bugs.debian.org/765717 > > > Signed-off-by: Ben Hutchings > > > > Any chance you can use Git instead of Quilt? Failing that, is there a > > way to tell Quilt to supply a diffstat in the patch? > > Why does it matter? To this patch? It probably doesn't. In general? It absolutely helps with patch review -- it's the second thing I look at. Diffstats are especially helpful if/when patches cross subsystem boundaries or when multiple files a touched. Until I scroll down I have no idea if this is just a memstick patch (which I can see at the bottom of my screen) or in fact amends 5 subsystems and requires special handling. > 'git am' is happy to apply it. But in case it > really does make a difference, I've attached the git-format-patch > version. I meant more 'from now on', rather than that patch being a special case. Ultimately it's your decision. I just think it makes things that little bit easier for the reviewer, as I'm sure you can appreciate. > > > --- > > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > > @@ -706,7 +706,8 @@ poll_again: > > > > > > > > > if (host->eject) > > > > > > > > > > > break; > > > > > > -> > > > > > msleep(1000); > > > +> > > > > > if (msleep_interruptible(1000)) > > > +> > > > > > > > flush_signals(current); > > > > > > > } > > > > > > > > > > complete(>detect_ms_exit); > > > > > > > From 34dca6b208a1108a9499412f584468b31ee025e0 Mon Sep 17 00:00:00 2001 > From: Ben Hutchings > Date: Sun, 26 Oct 2014 03:39:42 + > Subject: [PATCH] rtsx_usb_ms: Use msleep_interruptible() in polling loop > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings > --- > drivers/memstick/host/rtsx_usb_ms.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/memstick/host/rtsx_usb_ms.c > b/drivers/memstick/host/rtsx_usb_ms.c > index 1105db2..645dede 100644 > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Thu, 08 Oct 2015, Ben Hutchings wrote: > On Thu, 2015-10-08 at 08:19 +0100, Lee Jones wrote: > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). A load average of ~1 on a system that > > > should be idle is somewhat alarming. > > > > > > Change the sleep to be interruptible, but still ignore signals. > > > > > > A better fix might be to replace this loop with a delayed work item. > > > > > > References: https://bugs.debian.org/765717 > > > Signed-off-by: Ben Hutchings> > > > Any chance you can use Git instead of Quilt? Failing that, is there a > > way to tell Quilt to supply a diffstat in the patch? > > Why does it matter? To this patch? It probably doesn't. In general? It absolutely helps with patch review -- it's the second thing I look at. Diffstats are especially helpful if/when patches cross subsystem boundaries or when multiple files a touched. Until I scroll down I have no idea if this is just a memstick patch (which I can see at the bottom of my screen) or in fact amends 5 subsystems and requires special handling. > 'git am' is happy to apply it. But in case it > really does make a difference, I've attached the git-format-patch > version. I meant more 'from now on', rather than that patch being a special case. Ultimately it's your decision. I just think it makes things that little bit easier for the reviewer, as I'm sure you can appreciate. > > > --- > > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > > @@ -706,7 +706,8 @@ poll_again: > > > > > > > > > if (host->eject) > > > > > > > > > > > break; > > > > > > -> > > > > > msleep(1000); > > > +> > > > > > if (msleep_interruptible(1000)) > > > +> > > > > > > > flush_signals(current); > > > > > > > } > > > > > > > > > > complete(>detect_ms_exit); > > > > > > > From 34dca6b208a1108a9499412f584468b31ee025e0 Mon Sep 17 00:00:00 2001 > From: Ben Hutchings > Date: Sun, 26 Oct 2014 03:39:42 + > Subject: [PATCH] rtsx_usb_ms: Use msleep_interruptible() in polling loop > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings > --- > drivers/memstick/host/rtsx_usb_ms.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/memstick/host/rtsx_usb_ms.c > b/drivers/memstick/host/rtsx_usb_ms.c > index 1105db2..645dede 100644 > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Thu, 2015-10-08 at 08:19 +0100, Lee Jones wrote: > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). A load average of ~1 on a system that > > should be idle is somewhat alarming. > > > > Change the sleep to be interruptible, but still ignore signals. > > > > A better fix might be to replace this loop with a delayed work item. > > > > References: https://bugs.debian.org/765717 > > Signed-off-by: Ben Hutchings > > Any chance you can use Git instead of Quilt? Failing that, is there a > way to tell Quilt to supply a diffstat in the patch? Why does it matter? 'git am' is happy to apply it. But in case it really does make a difference, I've attached the git-format-patch version. Ben. > > --- > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > > >> > > > if (host->eject) > > > >> > > > > > break; > > > > -> >> > > > msleep(1000); > > +> >> > > > if (msleep_interruptible(1000)) > > +> >> > > > > > flush_signals(current); > > > >> > } > > > > > >> > complete(>detect_ms_exit); > > > -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.From 34dca6b208a1108a9499412f584468b31ee025e0 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Sun, 26 Oct 2014 03:39:42 + Subject: [PATCH] rtsx_usb_ms: Use msleep_interruptible() in polling loop rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben Hutchings --- drivers/memstick/host/rtsx_usb_ms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c index 1105db2..645dede 100644 --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 28 Sep 2015, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings Any chance you can use Git instead of Quilt? Failing that, is there a way to tell Quilt to supply a diffstat in the patch? > --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 28 Sep 2015, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben HutchingsAny chance you can use Git instead of Quilt? Failing that, is there a way to tell Quilt to supply a diffstat in the patch? > --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Thu, 2015-10-08 at 08:19 +0100, Lee Jones wrote: > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). A load average of ~1 on a system that > > should be idle is somewhat alarming. > > > > Change the sleep to be interruptible, but still ignore signals. > > > > A better fix might be to replace this loop with a delayed work item. > > > > References: https://bugs.debian.org/765717 > > Signed-off-by: Ben Hutchings> > Any chance you can use Git instead of Quilt? Failing that, is there a > way to tell Quilt to supply a diffstat in the patch? Why does it matter? 'git am' is happy to apply it. But in case it really does make a difference, I've attached the git-format-patch version. Ben. > > --- > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > > >> > > > if (host->eject) > > > >> > > > > > break; > > > > -> >> > > > msleep(1000); > > +> >> > > > if (msleep_interruptible(1000)) > > +> >> > > > > > flush_signals(current); > > > >> > } > > > > > >> > complete(>detect_ms_exit); > > > -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.From 34dca6b208a1108a9499412f584468b31ee025e0 Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Sun, 26 Oct 2014 03:39:42 + Subject: [PATCH] rtsx_usb_ms: Use msleep_interruptible() in polling loop rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben Hutchings --- drivers/memstick/host/rtsx_usb_ms.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/memstick/host/rtsx_usb_ms.c b/drivers/memstick/host/rtsx_usb_ms.c index 1105db2..645dede 100644 --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 2015-09-28 at 01:34 +0100, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. The change should be OK providing it does the same delay. You can add my ack. > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings > --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Best regards, Roger Tseng
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 2015-09-28 at 01:34 +0100, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. The change should be OK providing it does the same delay. You can add my ack. > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings> --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Best regards, Roger Tseng
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 28 Sep 2015, Ben Hutchings wrote: > On Mon, 2015-09-28 at 11:34 +0100, Lee Jones wrote: > > The subsystem is missing from the subject line. > > > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). A load average of ~1 on a system > > > that > > > should be idle is somewhat alarming. > > > > > > Change the sleep to be interruptible, but still ignore signals. > > > > > > A better fix might be to replace this loop with a delayed work > > > item. > > > > > > References: https://bugs.debian.org/765717 > > > Signed-off-by: Ben Hutchings > > > > How was this patch made? Where's the diff etc? > > With quilt. Old Skool! ;) > > Why was this patch sent to me? > > I think I must have added you previously as MFD maintainer, but as this > doesn't touch rtsx_pcr or rtsx_usb it's not really in your area. Sorry > to bother you. No worries. > > > --- > > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > > @@ -706,7 +706,8 @@ poll_again: > > > if (host->eject) > > > break; > > > > > > - msleep(1000); > > > + if (msleep_interruptible(1000)) > > > + flush_signals(current); > > > } > > > > > > complete(>detect_ms_exit); > > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 2015-09-28 at 11:34 +0100, Lee Jones wrote: > The subsystem is missing from the subject line. > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). A load average of ~1 on a system > > that > > should be idle is somewhat alarming. > > > > Change the sleep to be interruptible, but still ignore signals. > > > > A better fix might be to replace this loop with a delayed work > > item. > > > > References: https://bugs.debian.org/765717 > > Signed-off-by: Ben Hutchings > > How was this patch made? Where's the diff etc? With quilt. > Why was this patch sent to me? I think I must have added you previously as MFD maintainer, but as this doesn't touch rtsx_pcr or rtsx_usb it's not really in your area. Sorry to bother you. Ben. > > --- > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > if (host->eject) > > break; > > > > - msleep(1000); > > + if (msleep_interruptible(1000)) > > + flush_signals(current); > > } > > > > complete(>detect_ms_exit); > -- Ben Hutchings All extremists should be taken out and shot. signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
The subsystem is missing from the subject line. On Mon, 28 Sep 2015, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben Hutchings How was this patch made? Where's the diff etc? Why was this patch sent to me? > --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 2015-09-28 at 11:34 +0100, Lee Jones wrote: > The subsystem is missing from the subject line. > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > uninterruptible sleep still contribute to the load average (for > > bug-compatibility with Unix). A load average of ~1 on a system > > that > > should be idle is somewhat alarming. > > > > Change the sleep to be interruptible, but still ignore signals. > > > > A better fix might be to replace this loop with a delayed work > > item. > > > > References: https://bugs.debian.org/765717 > > Signed-off-by: Ben Hutchings> > How was this patch made? Where's the diff etc? With quilt. > Why was this patch sent to me? I think I must have added you previously as MFD maintainer, but as this doesn't touch rtsx_pcr or rtsx_usb it's not really in your area. Sorry to bother you. Ben. > > --- > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > @@ -706,7 +706,8 @@ poll_again: > > if (host->eject) > > break; > > > > - msleep(1000); > > + if (msleep_interruptible(1000)) > > + flush_signals(current); > > } > > > > complete(>detect_ms_exit); > -- Ben Hutchings All extremists should be taken out and shot. signature.asc Description: This is a digitally signed message part
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
On Mon, 28 Sep 2015, Ben Hutchings wrote: > On Mon, 2015-09-28 at 11:34 +0100, Lee Jones wrote: > > The subsystem is missing from the subject line. > > > > On Mon, 28 Sep 2015, Ben Hutchings wrote: > > > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > > > uninterruptible sleep still contribute to the load average (for > > > bug-compatibility with Unix). A load average of ~1 on a system > > > that > > > should be idle is somewhat alarming. > > > > > > Change the sleep to be interruptible, but still ignore signals. > > > > > > A better fix might be to replace this loop with a delayed work > > > item. > > > > > > References: https://bugs.debian.org/765717 > > > Signed-off-by: Ben Hutchings> > > > How was this patch made? Where's the diff etc? > > With quilt. Old Skool! ;) > > Why was this patch sent to me? > > I think I must have added you previously as MFD maintainer, but as this > doesn't touch rtsx_pcr or rtsx_usb it's not really in your area. Sorry > to bother you. No worries. > > > --- > > > --- a/drivers/memstick/host/rtsx_usb_ms.c > > > +++ b/drivers/memstick/host/rtsx_usb_ms.c > > > @@ -706,7 +706,8 @@ poll_again: > > > if (host->eject) > > > break; > > > > > > - msleep(1000); > > > + if (msleep_interruptible(1000)) > > > + flush_signals(current); > > > } > > > > > > complete(>detect_ms_exit); > > -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
The subsystem is missing from the subject line. On Mon, 28 Sep 2015, Ben Hutchings wrote: > rtsx_usb_ms creates a task that mostly sleeps, but tasks in > uninterruptible sleep still contribute to the load average (for > bug-compatibility with Unix). A load average of ~1 on a system that > should be idle is somewhat alarming. > > Change the sleep to be interruptible, but still ignore signals. > > A better fix might be to replace this loop with a delayed work item. > > References: https://bugs.debian.org/765717 > Signed-off-by: Ben HutchingsHow was this patch made? Where's the diff etc? Why was this patch sent to me? > --- > --- a/drivers/memstick/host/rtsx_usb_ms.c > +++ b/drivers/memstick/host/rtsx_usb_ms.c > @@ -706,7 +706,8 @@ poll_again: > if (host->eject) > break; > > - msleep(1000); > + if (msleep_interruptible(1000)) > + flush_signals(current); > } > > complete(>detect_ms_exit); -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben Hutchings --- --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); -- Ben Hutchings All extremists should be taken out and shot. signature.asc Description: This is a digitally signed message part
[PATCH RESEND] rtsx_usb_ms: Use msleep_interruptible() in polling loop
rtsx_usb_ms creates a task that mostly sleeps, but tasks in uninterruptible sleep still contribute to the load average (for bug-compatibility with Unix). A load average of ~1 on a system that should be idle is somewhat alarming. Change the sleep to be interruptible, but still ignore signals. A better fix might be to replace this loop with a delayed work item. References: https://bugs.debian.org/765717 Signed-off-by: Ben Hutchings--- --- a/drivers/memstick/host/rtsx_usb_ms.c +++ b/drivers/memstick/host/rtsx_usb_ms.c @@ -706,7 +706,8 @@ poll_again: if (host->eject) break; - msleep(1000); + if (msleep_interruptible(1000)) + flush_signals(current); } complete(>detect_ms_exit); -- Ben Hutchings All extremists should be taken out and shot. signature.asc Description: This is a digitally signed message part