Message-ID: <CACV3sbKHdzYEwap2r_o+4ccYAZU67qPkBYgQwNv3paDq8WJrnQ@mail.gmail.com>
Date:	Mon, 14 Nov 2011 13:49:04 +0800
From:	Jovi Zhang <bookjovi@...il.com>
To:	Neil Horman <nhorman@...driver.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Pádraig Brady <P@...igbrady.com>,
	dhowells@...hat.com, viro@...iv.linux.org.uk,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] coredump: fix pipe coredump when core limit is 0

On Wed, Aug 24, 2011 at 7:01 PM, Neil Horman <nhorman@...driver.com> wrote:
>
> On Wed, Aug 24, 2011 at 06:14:24PM +0800, Jovi Zhang wrote:
> > 2011/8/23 Oleg Nesterov <oleg@...hat.com>:
> > > On 08/22, Pádraig Brady wrote:
> > >>
> > >> On 08/21/2011 11:36 PM, Neil Horman wrote:
> > >> > Concur.  The comment should be changed
> > >> > Neil
> > >> >
> > >> > Oleg Nesterov <oleg@...hat.com> wrote:
> > >> >
> > >> >> On 08/21, Oleg Nesterov wrote:
> > >> >>>
> > >> >>> On 08/21, bookjovi@...il.com wrote:
> > >> >>>>
> > >> >>>> For non-pipe case, limit 0 also means drop the coredump, so just put
> > >> >>>> the zero limit check at do_coredump function begining.
> > >> >>>
> > >> >>> Neil, what do you think? Should we change the code or the comment?
> > >> >>
> > >> >> Personally I think we should fix the comment. I think RLIMIT_CORE
> > >> >> doesn't apply in this case, limit == 1 check is very special. And
> > >> >> this is what linux always did, except between 725eae32 and 898b374a.
> > >>
> > >> Sorry for jumping in late here.
> > >> I would really like `ulimit -c 0` to completely disable core dumps,
> > >> including not running core_pattern, as I also mentioned here:
> > >> https://bugs.launchpad.net/ubuntu/+source/apport/+bug/62511
> > >> I noticed this in a script where ctrl-\ was taking a long
> > >> time to be registered as the core_pattern was run unconditionally.
> > >
> > > May be. As I said, I do not really know and personally I agree with
> > > everything. My only point was, this is not the bug, this is what we
> > > always did.
> > >
> > > This is up to Neil, I think.
> > >
> > > Oleg.
> > >
> > >
> > Well, so here have two questions.
> > 1) That comments "but a limit of 0 skips the dump" definitely is wrong
> > right now, it don't match with reality.
> Agreed, I think your patch fixes this correctly.
>
> > 2) In ispipe case, core limit 0 should skip the dump or not? this need
> > more discussion.
> >    from pipe coredump point of view, core limit is irrelevant, it
> > doesn't write to file system.
> >    from user point of view, there will be a lot of core files if we
> > let core limit 0 create core file, user might be boring.
> >
> The case (ispipe==true && cprm.lmit==0) has to result in us dumping a core. I
> use to be convinced otherwise, but several user space developers changed my
> mind, particularly the guys writing the abrt daemon.  The reason being, the
> default process limit for RLIMIT_CORE is zero.  If you're writing a daemon like
> abrt that wants to catch program crashes, even during boot, there are tons of
> hoops you have to jump through to get core pipes enabled properly if you need to
> change RLIMIT_CORE.  Specifically you have to modify all existing processes
> RLIMIT_CORE values to be non-zero (a racy proposition) as well as modify the
> init processes RLIMIT_CORE value (so that it gets inherited by future
> processes).  Thats a pretty rickety thing to set up, and they really didn't want
> to have that much fiddling to do to get it all working, and I don't blame them.
> The fact that you're setting up a core pipe in the first place, implies to user
> space that you want an executed notification of cores, and in that execution you
> have the ability to filter which cores you actually care about.  If you're
> worried about too many processes spawning or getting the cpu bogged with crash
> handling, we have the core_limit sysctl to keep us throttled.
>
> The long and the short of it is, making RLIMIT_CORE == 0 for the ispipe case
> skip the core dump, breaks lots of user space expectations (which I know, is
> counter-intuitive), but changing it will open up a large can of worms, it works
> properly as it is.
>
> > I fix the comments part by below patch(thanks Oleg's comments), please
> > use attachment patch when merge.
> >
> > From dc7b02a1e0e413fb96d22f1d4ef4da98115cfb9d Mon Sep 17 00:00:00 2001
> > From: Jovi Zhang <bookjovi@...il.com>
> > Date: Wed, 17 Aug 2011 15:34:29 +0800
> > Subject: [PATCH] coredump: fix wrong comments on core limits of pipe
> > coredump case
> >
> > In commit 898b374a, core limits recursive check vaule changed from 0 to 1,
> > but the corresponding comments was not changed correctly.
> >
> > Signed-off-by: Jovi Zhang <bookjovi@...il.com>
> > Cc: Oleg Nesterov <oleg@...hat.com>
> > Cc: Neil Horman <nhorman@...driver.com>
> Reviewed-by: Neil Horman <nhorman@...driver.com>
>

Hi,
    How about this old patch? this patch still not upstreamed?
.jovi
--
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