LKML Archive on
help / color / mirror / Atom feed
From: "Kawai, Hidehiro" <>
To: Pavel Machek <>
Cc: Andrew Morton <>,
	kernel list <>,
	Robin Holt <>, David Howells <>,
	Alan Cox <>,
	Masami Hiramatsu <>,
	sugita <>,
	Satoshi OSHIMA <>,
	Hideo AOKI <>
Subject: Re: [PATCH 1/4] coredump: add an interface to control the core dump routine
Date: Mon, 26 Mar 2007 22:02:51 +0900	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hi Pavel, 
Thank you for your reply.
I'm sorry for my late reply.

I have discussed with my colleagues why you say "ugly" against my
procfs interface, then I noticed I may have misunderstood what you said.
Is the reason for saying "ugly" two interfaces, i.e. preexisting ulimit
(get/setrlimit) and my proc entry, exist to control core file size? 
If so, I'm sorry for taking your precious time by proceeding to the
discussion without enough understanding.

Assuming my presumption is true, I don't think it's so ugly because
there are other parameters to control core dumping such as dumpable
(controled by prctl(2)) and suid_dumpable (controled via
fs.suid_dumpable sysctl).
What would you think about that?

Anyway, here are the answers to what you pointed out.

Pavel Machek wrote:

>>This patch adds an interface to set/reset a flag which determines
>>anonymous shared memory segments should be dumped or not when a core
>>file is generated.
>>/proc/<pid>/coredump_omit_anonymous_shared file is provided to access
>>the flag. You can change the flag status for a particular process by
>>writing to or reading from the file.
> Yes. So, you used very different interface interface from ulimit,
> which means locking is hard.

I'd like to allow users to change the flag from other process.  So I
have to do locking even if it is hard.  You said this flexibility was
not an advantage before, but in some cases, it is needed.

Please assume the case where a process forks many children and they
share a huge shared memory.  Sometimes an end user wants to set 
coredump_omit_anon_shared flag to those processes except for a
particular child process.  With ulimit (setrlimit) interface,
the user can't do such setup without modifying the application
program.  But normal end user will not be able to modify the program.

> Plus, what you are doing can be done in userspace using google
> coredumper.

I think that the needs differ between userland core dumper user and
in-kernel core dumper user.  Pros and cons also differ.

Some of people (such as system admins, distro vendors, etc) need
highly reliable core dumper because they don't want to experience
same failures again and they don't hope that another failure is
caused by core dumping.  Userland core dumper is useful because
it is relatively easy to be customized, but its reliability highly
depends on the application programs.

If the stack for signal handlers is not set up carefully, if the
data used by userland core dumper has been destroyed, if
coredump_omit_anon_shared flag has been overwritten by bad data,
or if the address of functions have been destroyed, the userland
core dumper may fail to dump.  So in-kernel solutoin is required
by enterprise users.

>>+	if (down_write_trylock(&coredump_settings_sem)) {
>>+		set_coredump_omit_anon_shared(mm, (val != 0));
>>+		up_write(&coredump_settings_sem);
>>+	} else
>>+		ret = -EBUSY;
> Now this is an ugly interface. "If user tries to write to /proc file
> while it is locked, return him spurious error.

I'm considering using the previous argument passing approach (preserves
the setting value into a local variable and then passes it to core dump
routines) or another approach which introduce a per-process flag to
indicate that core dump is ongoing.  Both of these approach never
produce spurious errors.

>>@@ -75,6 +77,8 @@ extern int suid_dumpable;
>> #define SUID_DUMP_USER		1	/* Dump as user of process */
>> #define SUID_DUMP_ROOT		2	/* Dump as root */
>>+extern struct rw_semaphore coredump_settings_sem;
>> /* Stack area protections */
>> #define EXSTACK_DEFAULT   0	/* Whatever the arch defaults to */
>> #define EXSTACK_DISABLE_X 1	/* Disable executable stacks */
> Yep, very nice, if you used interface suited for the task (ulimit),
> you'd not have to invent new locking like this.

Using the above-stated approach, this semaphore becomes unnecessary.

Hidehiro Kawai
Hitachi, Ltd., Systems Development Laboratory

  reply	other threads:[~2007-03-26 13:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-02  4:41 [PATCH 0/4] coredump: core dump masking support v4 Kawai, Hidehiro
2007-03-02  4:47 ` [PATCH 1/4] coredump: add an interface to control the core dump routine Kawai, Hidehiro
2007-03-02  9:34   ` Pavel Machek
2007-03-26 13:02     ` Kawai, Hidehiro [this message]
2007-03-29 10:49       ` Pavel Machek
2007-03-29 19:16       ` David Howells
2007-03-29 21:17         ` Andrew Morton
2007-03-30 10:29           ` Kawai, Hidehiro
2007-03-30 16:10             ` Andrew Morton
2007-03-31 13:03             ` David Howells
2007-03-02  4:49 ` [PATCH 2/4] coredump: ELF: enable to omit anonymous shared memory Kawai, Hidehiro
2007-03-02  4:50 ` [PATCH 3/4] coredump: ELF-FDPIC: " Kawai, Hidehiro
2007-03-02  4:51 ` [PATCH 4/4] coredump: documentation for proc entry Kawai, Hidehiro
2007-03-02  9:35   ` Pavel Machek
2007-03-20 11:11     ` Kawai, Hidehiro
2007-03-15 20:37 ` [PATCH 0/4] coredump: core dump masking support v4 Andrew Morton
2007-03-23 13:13   ` Kawai, Hidehiro
2007-03-28 12:37     ` Kawai, Hidehiro
2007-03-28 17:32       ` Andrew Morton
  -- strict thread matches above, loose matches on Subject: below --
2007-02-16 13:34 [PATCH 0/4] coredump: core dump masking support v3 Kawai, Hidehiro
2007-02-16 13:39 ` [PATCH 1/4] coredump: add an interface to control the core dump routine Kawai, Hidehiro

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 1/4] coredump: add an interface to control the core dump routine' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).