LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: "Kawai, Hidehiro" <hidehiro.kawai.ez@hitachi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	kernel list <linux-kernel@vger.kernel.org>,
	Robin Holt <holt@sgi.com>, David Howells <dhowells@redhat.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	sugita <yumiko.sugita.yf@hitachi.com>,
	Satoshi OSHIMA <soshima@redhat.com>,
	Hideo AOKI <haoki@redhat.com>
Subject: Re: [PATCH 1/4] coredump: add an interface to control the core dump routine
Date: Fri, 2 Mar 2007 10:34:19 +0100	[thread overview]
Message-ID: <20070302093419.GA2001@elf.ucw.cz> (raw)
In-Reply-To: <45E7AC6F.8080704@hitachi.com>

Hi!

> 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.

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

> +	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.


> @@ -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.

Anyway, I don't care, and you don't listen. You got the design wrong,
and your code can be done in userspace, anyway. This is "NAK" from me,
but unfortunately I'm not in power to decide this. So at least drop me
from Cc in future submissions.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

  reply	other threads:[~2007-03-02  9:34 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 [this message]
2007-03-26 13:02     ` Kawai, Hidehiro
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:
  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=20070302093419.GA2001@elf.ucw.cz \
    --to=pavel@ucw.cz \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=dhowells@redhat.com \
    --cc=haoki@redhat.com \
    --cc=hidehiro.kawai.ez@hitachi.com \
    --cc=holt@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=soshima@redhat.com \
    --cc=yumiko.sugita.yf@hitachi.com \
    --subject='Re: [PATCH 1/4] coredump: add an interface to control the core dump routine' \
    /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

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).