[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20070315133721.ecbec123.akpm@linux-foundation.org>
Date: Thu, 15 Mar 2007 13:37:21 -0700
From: Andrew Morton <akpm@...ux-foundation.org>
To: "Kawai, Hidehiro" <hidehiro.kawai.ez@...achi.com>
Cc: kernel list <linux-kernel@...r.kernel.org>,
Pavel Machek <pavel@....cz>, Robin Holt <holt@....com>,
David Howells <dhowells@...hat.com>,
Alan Cox <alan@...rguk.ukuu.org.uk>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
sugita <yumiko.sugita.yf@...achi.com>,
Satoshi OSHIMA <soshima@...hat.com>,
Hideo AOKI <haoki@...hat.com>
Subject: Re: [PATCH 0/4] coredump: core dump masking support v4
On Fri, 02 Mar 2007 13:41:30 +0900
"Kawai, Hidehiro" <hidehiro.kawai.ez@...achi.com> wrote:
> This patch series is version 4 of the core dump masking feature,
> which provides a per-process flag not to dump anonymous shared
> memory segments.
First up, please convince us that this problem cannot be solved in
userspace. Note that we now support dumping core over a pipe to a
userspace application which can perform filtering such as this (see
Documentation/sysctl/kernel.txt:core_pattern).
Assuming that your argument is successful...
- The unpleasing trylock in proc_coredump_omit_anon_shared_write() is
there, I believe, to handle the case where a coredump is presently in
progress. We don't want to change the filtering rule while the dump is
happening.
What I suggest you do instead is to take a copy of
mm->coredump_omit_anon_shared into a local variable with one single read
per coredump. Pass that local down into all the callees which need to
see it. That way, no locking is needed.
- These games we're playing with the atomicity of the bitfields in the
mm_struct need to go away.
First up, please prepare a standalone patch which removes
mm_struct.dumpable and adds `unsigned long mm_struct.flags'. Include a
comment telling people that they must use atomic bitops (set_bit, clear_bit) on
mm_struct.flags.
Reimplement the current three-value dumpable silliness using two or
three separate flags in mm_struct.flags. Of course, this design means
that there will be tiny timing windows where the value of these two or
three flags have intermediate, invalid states. Please take care of those
little windows and document how you did so. I expect a suitable approach
would be to set and clear the flags in a suitable order, so that if a
race _does_ happen, the results are benign.
- Once that is done, you're ready to think about your new functionality.
Start out with
#define MM_FLAG_COREDUMP_OMIT_ANON_SHARED (1 << 3)
or whatever, and it all becomes easy.
- Finally, the code as you have it here is very specific to your specific
requirement: don't dump shared memory segments.
But if we're going to implement in-kernel core-dump filtering of this
nature, we should design it extensibly, even if we don't actually
implement those extensions at this time.
Because other people might (reasonably) wish to omit anonymous memory,
or private mappings, or file-backed VMAs, or whatever.
So maybe /proc/pid/coredump_omit_anon_shared should become
/proc/pid/core_dumpfilter, which is a carefully documented bitmask.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists