[PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded - Kernel

This is a discussion on [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded - Kernel ; If the coredumping is multi-threaded, format_corename() appends .%pid to the corename. This was needed before the proper multi-thread core dump support, now all the threads in the mm go into a single unified core file. Remove this special case, it ...

+ Reply to Thread
Results 1 to 6 of 6

Thread: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

  1. [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    If the coredumping is multi-threaded, format_corename() appends .%pid
    to the corename. This was needed before the proper multi-thread core
    dump support, now all the threads in the mm go into a single unified
    core file.

    Remove this special case, it is not even documented and we have "%p"
    and core_uses_pid.

    Signed-off-by: Oleg Nesterov

    --- 26-rc2/fs/exec.c~FORMAT_CORENAME_NO_MT_PID 2008-07-22 15:42:15.000000000 +0400
    +++ 26-rc2/fs/exec.c 2008-07-22 15:46:04.000000000 +0400
    @@ -1373,7 +1373,7 @@ EXPORT_SYMBOL(set_binfmt);
    * name into corename, which must have space for at least
    * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
    */
    -static int format_corename(char *corename, int nr_threads, long signr)
    +static int format_corename(char *corename, long signr)
    {
    const char *pat_ptr = core_pattern;
    int ispipe = (*pat_ptr == '|');
    @@ -1480,8 +1480,7 @@ static int format_corename(char *corenam
    * If core_pattern does not include a %p (as is the default)
    * and core_uses_pid is set, then .%pid will be appended to
    * the filename. Do not do this for piped commands. */
    - if (!ispipe && !pid_in_pattern
    - && (core_uses_pid || nr_threads)) {
    + if (!ispipe && !pid_in_pattern && core_uses_pid) {
    rc = snprintf(out_ptr, out_end - out_ptr,
    ".%d", task_tgid_vnr(current));
    if (rc > out_end - out_ptr)
    @@ -1745,7 +1744,7 @@ int do_coredump(long signr, int exit_cod
    * uses lock_kernel()
    */
    lock_kernel();
    - ispipe = format_corename(corename, retval, signr);
    + ispipe = format_corename(corename, signr);
    unlock_kernel();
    /*
    * Don't bother to check the RLIMIT_CORE value if core_pattern points

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    On Tue, Jul 22, 2008 at 2:18 PM, Oleg Nesterov wrote:
    > If the coredumping is multi-threaded, format_corename() appends .%pid
    > to the corename. This was needed before the proper multi-thread core
    > dump support, now all the threads in the mm go into a single unified
    > core file.
    >
    > Remove this special case, it is not even documented and we have "%p"
    > and core_uses_pid.


    Hi Oleg,

    I have not thought about this at any length, but one question that
    jumps to mind: could this feature still be useful for LinuxThreads,
    where each thread does indeed have a separate PID?

    Cheers,

    Michael

    >
    > Signed-off-by: Oleg Nesterov
    >
    > --- 26-rc2/fs/exec.c~FORMAT_CORENAME_NO_MT_PID 2008-07-22 15:42:15.000000000 +0400
    > +++ 26-rc2/fs/exec.c 2008-07-22 15:46:04.000000000 +0400
    > @@ -1373,7 +1373,7 @@ EXPORT_SYMBOL(set_binfmt);
    > * name into corename, which must have space for at least
    > * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
    > */
    > -static int format_corename(char *corename, int nr_threads, long signr)
    > +static int format_corename(char *corename, long signr)
    > {
    > const char *pat_ptr = core_pattern;
    > int ispipe = (*pat_ptr == '|');
    > @@ -1480,8 +1480,7 @@ static int format_corename(char *corenam
    > * If core_pattern does not include a %p (as is the default)
    > * and core_uses_pid is set, then .%pid will be appended to
    > * the filename. Do not do this for piped commands. */
    > - if (!ispipe && !pid_in_pattern
    > - && (core_uses_pid || nr_threads)) {
    > + if (!ispipe && !pid_in_pattern && core_uses_pid) {
    > rc = snprintf(out_ptr, out_end - out_ptr,
    > ".%d", task_tgid_vnr(current));
    > if (rc > out_end - out_ptr)
    > @@ -1745,7 +1744,7 @@ int do_coredump(long signr, int exit_cod
    > * uses lock_kernel()
    > */
    > lock_kernel();
    > - ispipe = format_corename(corename, retval, signr);
    > + ispipe = format_corename(corename, signr);
    > unlock_kernel();
    > /*
    > * Don't bother to check the RLIMIT_CORE value if core_pattern points
    >
    >




    --
    Michael Kerrisk
    Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
    man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
    Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    On 07/22, Michael Kerrisk wrote:
    >
    > On Tue, Jul 22, 2008 at 2:18 PM, Oleg Nesterov wrote:
    > > If the coredumping is multi-threaded, format_corename() appends .%pid
    > > to the corename. This was needed before the proper multi-thread core
    > > dump support, now all the threads in the mm go into a single unified
    > > core file.
    > >
    > > Remove this special case, it is not even documented and we have "%p"
    > > and core_uses_pid.

    >
    > Hi Oleg,
    >
    > I have not thought about this at any length, but one question that
    > jumps to mind: could this feature still be useful for LinuxThreads,
    > where each thread does indeed have a separate PID?


    As far as I know, LinuxThreads use CLONE_VM, right?

    The coredump will create the single core file for all processes because
    they have the same ->mm, the "threads" won't dump all over each other.

    And, just in case, this patch doesn't make any difference if core_uses_pid
    is set or pid_in_pattern is true.

    That said, this is the user-visible change...

    Oleg.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    On Tue, Jul 22, 2008 at 2:43 PM, Oleg Nesterov wrote:
    > On 07/22, Michael Kerrisk wrote:
    >>
    >> On Tue, Jul 22, 2008 at 2:18 PM, Oleg Nesterov wrote:
    >> > If the coredumping is multi-threaded, format_corename() appends .%pid
    >> > to the corename. This was needed before the proper multi-thread core
    >> > dump support, now all the threads in the mm go into a single unified
    >> > core file.
    >> >
    >> > Remove this special case, it is not even documented and we have "%p"
    >> > and core_uses_pid.

    >>
    >> Hi Oleg,
    >>
    >> I have not thought about this at any length, but one question that
    >> jumps to mind: could this feature still be useful for LinuxThreads,
    >> where each thread does indeed have a separate PID?

    >
    > As far as I know, LinuxThreads use CLONE_VM, right?


    Yes.

    > The coredump will create the single core file for all processes because
    > they have the same ->mm, the "threads" won't dump all over each other.


    Yes, looks like you are right. I had this vague idea that there were
    circumstances where a dump of a LinuxThreads m-t process could produce
    multipl core files, distinguished by the .PID, but I think I must have
    misremembered.

    > And, just in case, this patch doesn't make any difference if core_uses_pid
    > is set or pid_in_pattern is true.
    >
    > That said, this is the user-visible change...


    True. Not sure how important that is in this case though. What is
    the reason for making this change (other than tidiness)?

    --
    Michael Kerrisk
    Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
    man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
    Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    On 07/22, Michael Kerrisk wrote:
    >
    > On Tue, Jul 22, 2008 at 2:43 PM, Oleg Nesterov wrote:
    > >
    > > That said, this is the user-visible change...

    >
    > True. Not sure how important that is in this case though. What is
    > the reason for making this change (other than tidiness)?


    Tidiness is the only reason.

    Please don't hesitate to nack this patch if you think we shouldn't
    change the historical behaviour, the cleanup is very minor.

    As for me, I think it is a bit strange we append "%.pid" depending on
    is_multithreaded, the same app can have 1 or more threads for various
    reasons when the coredump happens, but this behaviour is very old and
    perhaps it is too late to change.

    Oleg.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH -mm] coredump: format_corename: don't append .%pid if multi-threaded

    On Tue, Jul 22, 2008 at 3:02 PM, Oleg Nesterov wrote:
    > On 07/22, Michael Kerrisk wrote:
    >>
    >> On Tue, Jul 22, 2008 at 2:43 PM, Oleg Nesterov wrote:
    >> >
    >> > That said, this is the user-visible change...

    >>
    >> True. Not sure how important that is in this case though. What is
    >> the reason for making this change (other than tidiness)?

    >
    > Tidiness is the only reason.
    >
    > Please don't hesitate to nack this patch if you think we shouldn't
    > change the historical behaviour, the cleanup is very minor.


    It is hard to think of something that might break because of this, so
    I'll remain silent.

    > As for me, I think it is a bit strange we append "%.pid" depending on
    > is_multithreaded, the same app can have 1 or more threads for various


    Agreed. It is strange.

    Cheers,

    Michael

    > reasons when the coredump happens, but this behaviour is very old and
    > perhaps it is too late to change.
    >
    > Oleg.
    >
    >




    --
    Michael Kerrisk
    Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
    man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
    Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread