(cc'ing Rafael and Oleg and quoting whole body)
On Thu, Feb 21, 2013 at 02:19:21PM +0800, Lianwei Wang wrote:
> Hi Tejun Heo and all,
>
> The commit of "34b087e freezer: kill unused
> set_freezable_with_signal()" remove recalc_sigpending*() calls in
> freezer, so the user tasks get TIF_SIGPENDING fake signal that is set
> when freezing userspace process. It left the fake signal to userspcae
> which cause the userspace task that wait_event_freezable and friends
> return a wrong ERESTARTSYS. This is not good because it waste cpu time
> to handle the fake signal.
Is this even measureable? Freeze / thaw isn't exactly a hot path and
I'm having difficult time believing -ERESTARTSYS would have a
noticeable impact on anything. Can you please explain why this is a
problem?
> Can we just call the recalc_sigpending to clear the fake signal for
> userspace tasks? as below patch do:
>
> From 176fccee178bc0185d92853dd2f521c9166b0853 Mon Sep 17 00:00:00 2001
> From: Lianwei Wang <lianwei.w...@gmail.com>
> Date: Mon, 21 Jan 2013 18:21:26 +0800
> Subject: [PATCH] freezer: add fake signal clearing back when thaw task
>
> The fake TIF_SIGPENDING is set during freeze userspace process, but it
> is not cleared when thaw tasks after below commit:
> 34b087e freezer: kill unused set_freezable_with_signal()
>
> This will cause the userspace task that wait_event_freezable and friends
> return a wrong ERESTARTSYS. This is not good because it waste cpu time to
> handle the fake signal.
>
> Try to clear the TIF_SIGPENDING flag for userspace apps when wakeup the
> frozen task to fix this issue.
>
> Change-Id: I91c90ad2ee9a46c42e3b39a7384ec81e97bc0394
> Signed-off-by: Lianwei Wang <lianwei.w...@gmail.com>
> ---
> kernel/freezer.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/freezer.c b/kernel/freezer.c
> index c38893b..09557f6 100644
> --- a/kernel/freezer.c
> +++ b/kernel/freezer.c
> @@ -46,6 +46,16 @@ bool freezing_slow_path(struct task_struct *p)
> }
> EXPORT_SYMBOL(freezing_slow_path);
>
> +static void fake_signal_clear(struct task_struct *p)
> +{
> + unsigned long flags;
> +
> + if (lock_task_sighand(p, &flags)) {
> + recalc_sigpending();
> + unlock_task_sighand(p, &flags);
> + }
> +}
> +
> /* Refrigerator is place where frozen processes are stored :-). */
> bool __refrigerator(bool check_kthr_stop)
> {
> @@ -74,6 +84,10 @@ bool __refrigerator(bool check_kthr_stop)
>
> pr_debug("%s left refrigerator\n", current->comm);
>
> + if (!(current->flags & PF_KTHREAD))
> + if (test_tsk_thread_flag(current, TIF_SIGPENDING))
> + fake_signal_clear(current);
> +
> /*
> * Restore saved task state before returning. The mb'd version
> * needs to be used; otherwise, it might silently break
> --
> 1.7.4.1
--
tejun
--
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/
