From: Andrew Morton <akpm@linux-foundation.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Mandeep Singh Baines <msb@chromium.org>,
Neil Horman <nhorman@redhat.com>,
"Rafael J. Wysocki" <rjw@sisk.pl>, Tejun Heo <tj@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] coredump: introduce dump_interrupted()
Date: Fri, 8 Mar 2013 13:20:46 -0800 [thread overview]
Message-ID: <20130308132046.02e2e6ac44a9fc4e63ed6604@linux-foundation.org> (raw)
In-Reply-To: <20130308175915.GA26322@redhat.com>
On Fri, 8 Mar 2013 18:59:15 +0100 Oleg Nesterov <oleg@redhat.com> wrote:
> Change dump_write(), dump_seek() and do_coredump() to check
> signal_pending() and abort if it is true.
hm, why.
I think we're missing some context here - this is to support freezing,
yes? There's some undescribed interaction between the freezer and the
core-dumper which is being fixed?
IOW, can we please have the high-level overview of what this patchset
is trying to achieve?
An example of why this is needed: the dump_interrupted() check which
was added to dump_seek() is just weird. An lseek is instantaneous, so
why do we need to bother checking for signals there if the caller will
be checking one microsecond later anyway??
And if the file doesn't support lseek (do such files exist? should we
be returning 0 instead of -ENOMEM?), we just sit there in a loop
extending the file with write(). This can take *ages*, but this part
of dump_seek() *didn't* get the signal check!
So it makes no sense at all. If we had that what-Oleg-is-trying-to-do
text then perhaps others could understand all of this?
> We add the new trivial helper, dump_interrupted(), to document that
> this probably needs more work and to simplify the potential freezer
> changes. Perhaps it will have more callers.
>
> Ideally it should do try_to_freeze() but then we need the unpleasant
> changes in dump_write() and wait_for_dump_helpers(). So far we simply
> accept the fact that the freezer can truncate a core-dump but at least
> you can reliably suspend.
OK, so there is some connection between this and suspending. Details,
please...
next prev parent reply other threads:[~2013-03-08 21:20 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-08 17:58 [PATCH -mm 0/3] coredump: signal_pending() checks and cleanups Oleg Nesterov
2013-03-08 17:59 ` [PATCH 1/3] coredump: introduce dump_interrupted() Oleg Nesterov
2013-03-08 20:54 ` Mandeep Singh Baines
2013-03-09 18:48 ` Oleg Nesterov
2013-03-08 21:20 ` Andrew Morton [this message]
2013-03-09 19:16 ` Oleg Nesterov
2013-03-08 17:59 ` [PATCH 2/3] coredump: factor out the setting of PF_DUMPCORE Oleg Nesterov
2013-03-08 20:21 ` Mandeep Singh Baines
2013-03-08 17:59 ` [PATCH 3/3] coredump: change wait_for_dump_helpers() to use wait_event_interruptible() Oleg Nesterov
2013-03-08 20:22 ` Mandeep Singh Baines
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130308132046.02e2e6ac44a9fc4e63ed6604@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=msb@chromium.org \
--cc=nhorman@redhat.com \
--cc=oleg@redhat.com \
--cc=rjw@sisk.pl \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome