diff mbox

[09/10] sigtimedwait: use freezable blocking call

Message ID 1367271946-7239-10-git-send-email-ccross@android.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Colin Cross April 29, 2013, 9:45 p.m. UTC
Avoid waking up every thread sleeping in a sigtimedwait call during
suspend and resume by calling a freezable blocking call.

Signed-off-by: Colin Cross <ccross@android.com>
---
 kernel/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Oleg Nesterov April 30, 2013, 4:38 p.m. UTC | #1
On 04/29, Colin Cross wrote:
>
> Avoid waking up every thread sleeping in a sigtimedwait call during
> suspend and resume by calling a freezable blocking call.

This doesn't explain why do want this change...

OK, probably to avoid -EAGAIN from sigtimedwait() if the freezer wakes
up the caller.

> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2845,7 +2845,7 @@ int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
>  		recalc_sigpending();
>  		spin_unlock_irq(&tsk->sighand->siglock);
>
> -		timeout = schedule_timeout_interruptible(timeout);
> +		timeout = freezable_schedule_timeout_interruptible(timeout);

And I guess freezable_schedule_timeout_interruptible() is added by
http://marc.info/?l=linux-kernel&m=136727195719575 ...

	+#define freezable_schedule_timeout_interruptible(timeout)		\
	+({									\
	+	long __retval;							\
	+	freezer_do_not_count();						\
	+	__retval = schedule_timeout_interruptible(timeout);		\
	+	freezer_count();						\
	+	__retval;							\
	+})

How this can help?

The task will be interrupted anyway and the syscall will return
-EAGAIN, this only changes the time when try_to_freeze() is called.

For what? The task will call do_signal/try_to_freeze really "soon".

Confused...

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Oleg Nesterov April 30, 2013, 4:56 p.m. UTC | #2
On 04/30, Oleg Nesterov wrote:
>
> On 04/29, Colin Cross wrote:
> >
> > Avoid waking up every thread sleeping in a sigtimedwait call during
> > suspend and resume by calling a freezable blocking call.
>
> This doesn't explain why do want this change...
>
> OK, probably to avoid -EAGAIN from sigtimedwait() if the freezer wakes
> up the caller.
>
> > --- a/kernel/signal.c
> > +++ b/kernel/signal.c
> > @@ -2845,7 +2845,7 @@ int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
> >  		recalc_sigpending();
> >  		spin_unlock_irq(&tsk->sighand->siglock);
> >
> > -		timeout = schedule_timeout_interruptible(timeout);
> > +		timeout = freezable_schedule_timeout_interruptible(timeout);
>
> And I guess freezable_schedule_timeout_interruptible() is added by
> http://marc.info/?l=linux-kernel&m=136727195719575 ...
>
> 	+#define freezable_schedule_timeout_interruptible(timeout)		\
> 	+({									\
> 	+	long __retval;							\
> 	+	freezer_do_not_count();						\
> 	+	__retval = schedule_timeout_interruptible(timeout);		\
> 	+	freezer_count();						\
> 	+	__retval;							\
> 	+})
>
> How this can help?
>
> The task will be interrupted anyway and the syscall will return
> -EAGAIN, this only changes the time when try_to_freeze() is called.

OK, I wasn't cced, I have found another patch
http://marc.info/?l=linux-kernel&m=136727200519602 which should make
a difference, it won't be woken if PF_FREEZER_SKIP was already set.

This is racy, but it seems that "avoid -EAGAIN" was not your goal...

> For what? The task will call do_signal/try_to_freeze really "soon".

It seems that you want to speed up the freezing.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Colin Cross April 30, 2013, 4:58 p.m. UTC | #3
On Tue, Apr 30, 2013 at 9:38 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 04/29, Colin Cross wrote:
>>
>> Avoid waking up every thread sleeping in a sigtimedwait call during
>> suspend and resume by calling a freezable blocking call.
>
> This doesn't explain why do want this change...
>
> OK, probably to avoid -EAGAIN from sigtimedwait() if the freezer wakes
> up the caller.

See http://marc.info/?l=linux-pm&m=136727197819593&w=2 for the full
justification.  I will include a fuller description of the reason for
this patch in the next version.

>> --- a/kernel/signal.c
>> +++ b/kernel/signal.c
>> @@ -2845,7 +2845,7 @@ int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
>>               recalc_sigpending();
>>               spin_unlock_irq(&tsk->sighand->siglock);
>>
>> -             timeout = schedule_timeout_interruptible(timeout);
>> +             timeout = freezable_schedule_timeout_interruptible(timeout);
>
> And I guess freezable_schedule_timeout_interruptible() is added by
> http://marc.info/?l=linux-kernel&m=136727195719575 ...
>
>         +#define freezable_schedule_timeout_interruptible(timeout)              \
>         +({                                                                     \
>         +       long __retval;                                                  \
>         +       freezer_do_not_count();                                         \
>         +       __retval = schedule_timeout_interruptible(timeout);             \
>         +       freezer_count();                                                \
>         +       __retval;                                                       \
>         +})
>
> How this can help?
>
> The task will be interrupted anyway and the syscall will return
> -EAGAIN, this only changes the time when try_to_freeze() is called.
>
> For what? The task will call do_signal/try_to_freeze really "soon".

See http://marc.info/?l=linux-pm&m=136727204919622&w=2, which removes
the wakeup sent to skipped tasks, so schedule_timeout_interruptible()
will only return if the timeout finishes or another task (not the
freezer) calls wake_up on the task.  If that happens during freezing
or while frozen freezer_count() will freeze.  If another task does not
wake up this task while frozen, this task will not need to run at all
during suspend or resume, saving cpu cycles, context switches, and
power.

> Confused...
>
> Oleg.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Oleg Nesterov April 30, 2013, 5 p.m. UTC | #4
On 04/30, Colin Cross wrote:
> On Tue, Apr 30, 2013 at 9:38 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > On 04/29, Colin Cross wrote:
> >>
> >> Avoid waking up every thread sleeping in a sigtimedwait call during
> >> suspend and resume by calling a freezable blocking call.
> >
> > This doesn't explain why do want this change...
> >
> > OK, probably to avoid -EAGAIN from sigtimedwait() if the freezer wakes
> > up the caller.
>
> See http://marc.info/?l=linux-pm&m=136727197819593&w=2 for the full
> justification.  I will include a fuller description of the reason for
> this patch in the next version.

Yes, thanks, I already realized what are you trying to do.

> > The task will be interrupted anyway and the syscall will return
> > -EAGAIN, this only changes the time when try_to_freeze() is called.
> >
> > For what? The task will call do_signal/try_to_freeze really "soon".
>
> See http://marc.info/?l=linux-pm&m=136727204919622&w=2, which removes
> the wakeup sent to skipped tasks, so schedule_timeout_interruptible()
> will only return if the timeout finishes or another task

Or if freeze_task() was already called.

but I guess you do not care.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/kernel/signal.c b/kernel/signal.c
index 598dc06..10a70a0 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2845,7 +2845,7 @@  int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
 		recalc_sigpending();
 		spin_unlock_irq(&tsk->sighand->siglock);
 
-		timeout = schedule_timeout_interruptible(timeout);
+		timeout = freezable_schedule_timeout_interruptible(timeout);
 
 		spin_lock_irq(&tsk->sighand->siglock);
 		__set_task_blocked(tsk, &tsk->real_blocked);