On Tue, Feb 26, 2013 at 8:37 AM, Mandeep Singh Baines <m...@chromium.org> wrote:
> On Sun, Feb 24, 2013 at 10:36 AM, Oleg Nesterov <o...@redhat.com> wrote:
>> A coredumping thread can't be frozen anyway but the fake signal sent
>> by freeze_task() can confuse dump_write/wait_for_dump_helpers/etc
>> and interrupt the coredump.
>>
>> We are going to make the do_coredump() paths freezable but the fake
>> TIF_SIGPENDING doesn't help, it only makes sense when we assume that
>> the target can return to user-mode and call get_signal_to_deliver().
>>
>> Change freeze_task() to check PF_DUMPCORE along with PF_KTHREAD. We
>> need to recheck PF_DUMPCORE under ->siglock to avoid the race with
>> zap_threads() which can set this flag right before we take the lock.
>>
>
> Won't this prevent suspend?
>
> If there is a wait_event_interruptible in the coredump path, you'll
> wake it up but it will simply go back to sleep. So try_to_freeze_tasks
> will fail waiting for the coredump thread to enter the freezer.
>
You'd rather have reliable suspend than coredumps that aren't
truncated so you need to set TIF_SIGPENDING to break waits in the
dump_write path.
But it would be nice to have both so you'd like to avoid terminating
on a signal. I think you'll need to fix each wait independent by
making it freezable or adding try_to_freeze?
To fix wait_for_dump_helpers:
static void wait_for_dump_helpers(struct file *file)
{
struct pipe_inode_info *pipe;
pipe = file->f_path.dentry->d_inode->i_pipe;
pipe_lock(pipe);
pipe->readers++;
pipe->writers--;
while (pipe->readers > 1) {
unsigned long flags;
wake_up_interruptible_sync(&pipe->wait);
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
pipe_wait(pipe);
pipe_unlock(pipe);
try_to_freeze();
pipe_lock(pipe);
if (fatal_signal_pending(current))
break;
/* Clear fake signal from freeze_task(). */
spin_lock_irqsave(¤t->sighand->siglock, flags);
recalc_sigpending();
spin_unlock_irqrestore(¤t->sighand->siglock, flags);
}
pipe->readers--;
pipe->writers++;
pipe_unlock(pipe);
}
What do you think? That would fix most cases. You'll still get a
truncated core if you were to receive the signal during pipe_write or
something.
Regards,
Mandeep
> Regards,
> Mandeep
>
>> Signed-off-by: Oleg Nesterov <o...@redhat.com>
>> ---
>> kernel/freezer.c | 19 ++++++++++++-------
>> 1 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/freezer.c b/kernel/freezer.c
>> index c38893b..595afab 100644
>> --- a/kernel/freezer.c
>> +++ b/kernel/freezer.c
>> @@ -85,14 +85,21 @@ bool __refrigerator(bool check_kthr_stop)
>> }
>> EXPORT_SYMBOL(__refrigerator);
>>
>> -static void fake_signal_wake_up(struct task_struct *p)
>> +static bool fake_signal_wake_up(struct task_struct *p)
>> {
>> unsigned long flags;
>> + bool ret = false;
>> +
>> + if (p->flags & (PF_KTHREAD | PF_DUMPCORE))
>> + return ret;
>>
>> if (lock_task_sighand(p, &flags)) {
>> - signal_wake_up(p, 0);
>> + ret = !(p->flags & PF_DUMPCORE);
>> + if (ret)
>> + signal_wake_up(p, 0);
>> unlock_task_sighand(p, &flags);
>> }
>> + return ret;
>> }
>>
>> /**
>> @@ -100,8 +107,8 @@ static void fake_signal_wake_up(struct task_struct *p)
>> * @p: task to send the request to
>> *
>> * If @p is freezing, the freeze request is sent either by sending a fake
>> - * signal (if it's not a kernel thread) or waking it up (if it's a kernel
>> - * thread).
>> + * signal (if it's not a kernel thread or a coredumping thread) or waking
>> + * it up otherwise.
>> *
>> * RETURNS:
>> * %false, if @p is not freezing or already frozen; %true, otherwise
>> @@ -116,9 +123,7 @@ bool freeze_task(struct task_struct *p)
>> return false;
>> }
>>
>> - if (!(p->flags & PF_KTHREAD))
>> - fake_signal_wake_up(p);
>> - else
>> + if (!fake_signal_wake_up(p))
>> wake_up_state(p, TASK_INTERRUPTIBLE);
>>
>> spin_unlock_irqrestore(&freezer_lock, flags);
>> --
>> 1.5.5.1
>>
>>
--
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/
