From: R Nageswara Sastry <rnsastry@linux.ibm.com>
To: Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org
Cc: Jacob Lalonde <jalalonde@meta.com>,
Josef Bacik <josef@toxicpanda.com>, Jann Horn <jannh@google.com>,
Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.cz>,
Andrew Morton <akpm@linux-foundation.org>,
David Hildenbrand <david@kernel.org>,
Lorenzo Stoakes <ljs@kernel.org>,
"Liam R. Howlett" <liam@infradead.org>,
Vlastimil Babka <vbabka@kernel.org>,
Mike Rapoport <rppt@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Omar Sandoval <osandov@osandov.com>,
Jacob Lalonde <jalalonde@fb.com>, Shuah Khan <shuah@kernel.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linux-kselftest@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 12/22] coredump: clean up coredump state handling
Date: Wed, 9 Sep 2026 21:31:31 +0530 [thread overview]
Message-ID: <0e07e597-d816-4425-b190-d61c3281f95c@linux.ibm.com> (raw)
In-Reply-To: <20260820-work-coredump-sparse-v2-12-ba32dd718c51@kernel.org>
On 20.08.2026 4:39 AM, Christian Brauner wrote:
> Right now coredump state handling is messy. The binfmt->core_dump::
> methods return 1 when the coredump method did anything at all which
> means that a partial write counts as having dumped core. This is fine as
> the state is really only used to indicate that a coredump event occurred
> in the exit status of the task. That should obviously be indicated even
> if the actual writeout of the coredump failed.
>
> But it means the coredump method of the binary formats is different from
> all the other coredump helpers. And there's no way to communicate to
> userspace that a coredump was truncated. We'll add support for that in a
> second. For now, clean this up.
>
> Add a flag member into struct coredump_params. Let the coredump method
> raise COREDUMP_STATE_STARTED. This is what coredump_finish() will end up
> using to splice in the coredump bit into the exit status.
>
> This allows us to let the return value mean success or failure and align
> it with the other coredump helpers. We also start raising
> COREDUMP_STATE_TRUNCATED. This will be used in the next patches to
> communicate truncation to userspace via the coredump socket.
>
> No functional changes.
>
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Tested-by: R Nageswara Sastry <rnsastry@linux.ibm.com>
System: ppc64le LPAR (IBM POWER), Linux 7.3-rc2
> ---
> fs/binfmt_elf.c | 12 +++++++-----
> fs/binfmt_elf_fdpic.c | 12 +++++++-----
> fs/coredump.c | 31 +++++++++++++++++--------------
> include/linux/binfmts.h | 3 ++-
> include/linux/coredump.h | 12 ++++++++++++
> 5 files changed, 45 insertions(+), 25 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 16a56b6b3f6c..f85b2137bdb9 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -74,7 +74,7 @@ static int load_elf_binary(struct linux_binprm *bprm);
> * don't even try.
> */
> #ifdef CONFIG_ELF_CORE
> -static int elf_core_dump(struct coredump_params *cprm);
> +static bool elf_core_dump(struct coredump_params *cprm);
> #else
> #define elf_core_dump NULL
> #endif
> @@ -1990,9 +1990,9 @@ static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
> * and then they are actually written out. If we run out of core limit
> * we just truncate.
> */
> -static int elf_core_dump(struct coredump_params *cprm)
> +static bool elf_core_dump(struct coredump_params *cprm)
> {
> - int has_dumped = 0;
> + bool ret = false;
> int segs, i;
> struct elfhdr elf;
> loff_t offset = 0, dataoff;
> @@ -2023,7 +2023,7 @@ static int elf_core_dump(struct coredump_params *cprm)
> if (!fill_note_info(&elf, e_phnum, &info, cprm))
> goto end_coredump;
>
> - has_dumped = 1;
> + cprm->state |= COREDUMP_STATE_STARTED;
>
> offset += sizeof(elf); /* ELF header */
> offset += segs * sizeof(struct elf_phdr); /* Program headers */
> @@ -2118,11 +2118,13 @@ static int elf_core_dump(struct coredump_params *cprm)
> goto end_coredump;
> }
>
> + ret = true;
> +
> end_coredump:
> free_note_info(&info);
> kfree(shdr4extnum);
> kfree(phdr4note);
> - return has_dumped;
> + return ret;
> }
>
> #endif /* CONFIG_ELF_CORE */
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index fe0b5c5ed2bc..252eb155965b 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -75,7 +75,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *,
> struct file *, struct mm_struct *);
>
> #ifdef CONFIG_ELF_CORE
> -static int elf_fdpic_core_dump(struct coredump_params *cprm);
> +static bool elf_fdpic_core_dump(struct coredump_params *cprm);
> #endif
>
> static struct linux_binfmt elf_fdpic_format = {
> @@ -1476,9 +1476,9 @@ static bool elf_fdpic_dump_segments(struct coredump_params *cprm,
> * and then they are actually written out. If we run out of core limit
> * we just truncate.
> */
> -static int elf_fdpic_core_dump(struct coredump_params *cprm)
> +static bool elf_fdpic_core_dump(struct coredump_params *cprm)
> {
> - int has_dumped = 0;
> + bool ret = false;
> int segs;
> int i;
> struct elfhdr *elf = NULL;
> @@ -1535,7 +1535,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
> /* Set up header */
> fill_elf_fdpic_header(elf, e_phnum);
>
> - has_dumped = 1;
> + cprm->state |= COREDUMP_STATE_STARTED;
> /*
> * Set up the notes in similar form to SVR4 core dumps made
> * with info from their /proc.
> @@ -1655,6 +1655,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
> cprm->file->f_pos, offset);
> }
>
> + ret = true;
> +
> end_coredump:
> while (thread_list) {
> tmp = thread_list;
> @@ -1665,7 +1667,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
> kfree(elf);
> kfree(psinfo);
> kfree(shdr4extnum);
> - return has_dumped;
> + return ret;
> }
>
> #endif /* CONFIG_ELF_CORE */
> diff --git a/fs/coredump.c b/fs/coredump.c
> index d61f36239f91..6af3ff0e19a6 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -98,7 +98,6 @@ struct core_name {
> char *corename __counted_by_ptr(size);
> int used, size;
> unsigned int core_pipe_limit;
> - bool core_dumped;
> enum coredump_type_t core_type;
> };
>
> @@ -250,7 +249,6 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
> cn->used = 0;
> cn->corename = NULL;
> cn->core_pipe_limit = 0;
> - cn->core_dumped = false;
> if (*pat_ptr == '|')
> cn->core_type = COREDUMP_PIPE;
> else if (*pat_ptr == '@')
> @@ -549,13 +547,13 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
> return core_waiters;
> }
>
> -static void coredump_finish(bool core_dumped)
> +static void coredump_finish(enum coredump_state state)
> {
> struct core_thread *curr, *next;
> struct task_struct *task;
>
> spin_lock_irq(¤t->sighand->siglock);
> - if (core_dumped && !__fatal_signal_pending(current))
> + if ((state & COREDUMP_STATE_STARTED) && !__fatal_signal_pending(current))
> current->signal->group_exit_code |= 0x80;
> next = current->signal->core_state->dumper.next;
> current->signal->core_state = NULL;
> @@ -1045,19 +1043,23 @@ static bool coredump_pipe(struct core_name *cn, struct coredump_params *cprm,
> return true;
> }
>
> -static bool coredump_write(struct core_name *cn,
> - struct coredump_params *cprm,
> - const struct linux_binfmt *binfmt)
> +static bool coredump_write(struct coredump_params *cprm,
> + const struct linux_binfmt *binfmt)
> {
>
> - if (dump_interrupted())
> + if (dump_interrupted()) {
> + cprm->state |= COREDUMP_STATE_TRUNCATED;
> return true;
> + }
>
> - if (!dump_vma_snapshot(cprm))
> + if (!dump_vma_snapshot(cprm)) {
> + cprm->state |= COREDUMP_STATE_TRUNCATED;
> return false;
> + }
>
> file_start_write(cprm->file);
> - cn->core_dumped = binfmt->core_dump(cprm);
> + if (!binfmt->core_dump(cprm))
> + cprm->state |= COREDUMP_STATE_TRUNCATED;
> /*
> * Ensures that file size is big enough to contain the current
> * file postion. This prevents gdb from complaining about
> @@ -1066,7 +1068,8 @@ static bool coredump_write(struct core_name *cn,
> */
> if (cprm->to_skip) {
> cprm->to_skip--;
> - dump_emit(cprm, "", 1);
> + if (!dump_emit(cprm, "", 1))
> + cprm->state |= COREDUMP_STATE_TRUNCATED;
> }
> file_end_write(cprm->file);
> free_vma_snapshot(cprm);
> @@ -1082,7 +1085,7 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
> atomic_dec(&core_pipe_count);
> }
> kfree(cn->corename);
> - coredump_finish(cn->core_dumped);
> + coredump_finish(cprm->state);
> }
>
> static inline bool coredump_skip(const struct coredump_params *cprm,
> @@ -1136,14 +1139,14 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
> if (unshare_files())
> return;
>
> - if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cn, cprm, binfmt))
> + if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cprm, binfmt))
> return;
>
> coredump_sock_shutdown(cprm->file);
>
> /* Let the parent know that a coredump was generated. */
> if (cprm->mask & COREDUMP_USERSPACE)
> - cn->core_dumped = true;
> + cprm->state |= COREDUMP_STATE_STARTED;
>
> /*
> * When core_pipe_limit is set we wait for the coredump server
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 2c77e383e737..64f82fb642bf 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -94,7 +94,8 @@ struct linux_binfmt {
> struct module *module;
> int (*load_binary)(struct linux_binprm *);
> #ifdef CONFIG_COREDUMP
> - int (*core_dump)(struct coredump_params *cprm);
> + /* Returns true if the whole coredump was written. */
> + bool (*core_dump)(struct coredump_params *cprm);
> unsigned long min_coredump; /* minimal dump size */
> #endif
> } __randomize_layout;
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index 943bddfb22bf..709388dd5659 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -9,6 +9,16 @@
> #include <asm/siginfo.h>
>
> #ifdef CONFIG_COREDUMP
> +/**
> + * enum coredump_state - what happened while the coredump was written
> + * @COREDUMP_STATE_STARTED: the dumper committed to writing a coredump
> + * @COREDUMP_STATE_TRUNCATED: the dumper stopped before it had written all of it
> + */
> +enum coredump_state {
> + COREDUMP_STATE_STARTED = (1U << 0),
> + COREDUMP_STATE_TRUNCATED = (1U << 1),
> +};
> +
> struct core_vma_metadata {
> unsigned long start, end;
> vm_flags_t flags;
> @@ -28,6 +38,8 @@ struct coredump_params {
> int cpu;
> /* COREDUMP_* options negotiated with the coredump server. */
> u64 mask;
> + /* COREDUMP_STATE_* raised while the coredump is written. */
> + enum coredump_state state;
> loff_t written;
> loff_t pos;
> loff_t to_skip;
>
--
Thanks and Regards
R.Nageswara Sastry
next prev parent reply other threads:[~2026-09-09 16:02 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-19 23:09 [PATCH v2 00/22] coredump: allow to create sparse coredumps on the coredump socket Christian Brauner
2026-08-19 23:09 ` [PATCH v2 01/22] powerpc/spufs: don't dump more than the note supports Christian Brauner
2026-09-09 15:52 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 02/22] coredump: refuse negative skips Christian Brauner
2026-09-09 15:53 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 03/22] coredump: set the minimum send buffer size Christian Brauner
2026-09-09 15:53 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 04/22] selftests/coredump: discard the right amount after the coredump request Christian Brauner
2026-09-09 15:54 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 05/22] selftests/coredump: collapse the expected request check into the helper Christian Brauner
2026-09-09 15:54 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 06/22] selftests/coredump: add a separate helper header Christian Brauner
2026-09-09 15:55 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 07/22] coredump: pin the protocol struct sizes Christian Brauner
2026-09-09 15:55 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 08/22] coredump: move the negotiated mask into struct coredump_params Christian Brauner
2026-09-09 15:56 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 09/22] coredump: deduplicate the to_skip flush Christian Brauner
2026-09-09 15:56 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 10/22] coredump: make the dump helper return bool Christian Brauner
2026-09-09 15:59 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 11/22] coredump: always chunk writes Christian Brauner
2026-09-09 16:00 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 12/22] coredump: clean up coredump state handling Christian Brauner
2026-09-09 16:01 ` R Nageswara Sastry [this message]
2026-08-19 23:09 ` [PATCH v2 13/22] coredump: add COREDUMP_RECORDS to the coredump socket protocol Christian Brauner
2026-09-09 16:02 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 14/22] coredump: add COREDUMP_SPARSE " Christian Brauner
2026-09-09 16:02 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 15/22] tools: sync coredump.h header Christian Brauner
2026-09-09 16:03 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 16/22] coredump: send the coredump in records if requested Christian Brauner
2026-09-09 16:04 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 17/22] coredump: describe the holes when COREDUMP_SPARSE is negotiated Christian Brauner
2026-09-09 16:04 ` R Nageswara Sastry
2026-09-09 16:21 ` David Hildenbrand (Arm)
2026-08-19 23:09 ` [PATCH v2 18/22] selftests/coredump: test COREDUMP_RECORDS and COREDUMP_SPARSE Christian Brauner
2026-09-09 16:05 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 19/22] selftests/coredump: hand the record stream to a sink Christian Brauner
2026-09-09 16:05 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 20/22] selftests/coredump: put a hole in the middle of a sparse mapping Christian Brauner
2026-09-09 16:06 ` R Nageswara Sastry
2026-09-09 16:18 ` David Hildenbrand (Arm)
2026-08-19 23:09 ` [PATCH v2 21/22] selftests/coredump: simulate a blob store Christian Brauner
2026-09-09 16:06 ` R Nageswara Sastry
2026-08-19 23:09 ` [PATCH v2 22/22] selftests/coredump: show how to inspect the task to decide how the coredump should be sent Christian Brauner
2026-09-09 16:07 ` R Nageswara Sastry
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=0e07e597-d816-4425-b190-d61c3281f95c@linux.ibm.com \
--to=rnsastry@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=david@kernel.org \
--cc=jack@suse.cz \
--cc=jalalonde@fb.com \
--cc=jalalonde@meta.com \
--cc=jannh@google.com \
--cc=josef@toxicpanda.com \
--cc=liam@infradead.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=ljs@kernel.org \
--cc=mhocko@suse.com \
--cc=osandov@osandov.com \
--cc=rppt@kernel.org \
--cc=shuah@kernel.org \
--cc=surenb@google.com \
--cc=vbabka@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox