linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 16/22] coredump: send the coredump in records if requested
Date: Wed, 9 Sep 2026 21:34:21 +0530	[thread overview]
Message-ID: <c578bc04-3128-426a-bedb-b53155510f55@linux.ibm.com> (raw)
In-Reply-To: <20260820-work-coredump-sparse-v2-16-ba32dd718c51@kernel.org>


On 20.08.2026 4:39 AM, Christian Brauner wrote:
> When the coredump server raises COREDUMP_RECORDS send the coredump in
> records. A record consists of a struct coredump_record_header and data.
> A header and the bytes it describes go out in one iovec.
>
> A hole is flushed through __dump_emit() like before. So zeroes still are
> sent on the socket as actual data records. Making holes cheap is
> COREDUMP_SPARSE's job.
>
> 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/coredump.c                                      | 150 +++++++++++++++++----
>   include/linux/coredump.h                           |   5 +
>   .../selftests/coredump/coredump_test_helpers.c     |   2 +-
>   3 files changed, 127 insertions(+), 30 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 6af3ff0e19a6..b1679930094c 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -51,7 +51,6 @@
>   #include <net/sock.h>
>   #include <uapi/linux/pidfd.h>
>   #include <uapi/linux/un.h>
> -#include <uapi/linux/coredump.h>
>   
>   #include <linux/uaccess.h>
>   #include <asm/mmu_context.h>
> @@ -68,6 +67,7 @@
>   
>   static bool dump_vma_snapshot(struct coredump_params *cprm);
>   static void free_vma_snapshot(struct coredump_params *cprm);
> +static void dump_end_record(struct coredump_params *cprm);
>   
>   #define CORE_FILE_NOTE_SIZE_DEFAULT (4*1024*1024)
>   /* Define a reasonable max cap */
> @@ -661,6 +661,8 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
>   	return 0;
>   }
>   
> +static_assert(sizeof(struct coredump_record_header) == COREDUMP_RECORD_HEADER_SIZE_VER0);
> +
>   #ifdef CONFIG_UNIX
>   /* af_unix halves the send buffer to size a single skb. */
>   #define COREDUMP_SOCK_SNDBUF_MIN (3 * PAGE_SIZE)
> @@ -803,7 +805,8 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
>   	struct coredump_req req = {
>   		.size		= sizeof(struct coredump_req),
>   		.mask		= COREDUMP_KERNEL | COREDUMP_USERSPACE |
> -				  COREDUMP_REJECT | COREDUMP_WAIT,
> +				  COREDUMP_REJECT | COREDUMP_WAIT |
> +				  COREDUMP_RECORDS,
>   		.size_ack	= sizeof(struct coredump_ack),
>   	};
>   	struct coredump_ack ack = {};
> @@ -857,6 +860,19 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
>   		return false;
>   	}
>   
> +	/* Records only describe a coredump the kernel writes. */
> +	if ((ack.mask & COREDUMP_RECORDS) && !(ack.mask & COREDUMP_KERNEL)) {
> +		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
> +		return false;
> +	}
> +
> +	/* Record header scratch; a bvec can't point at the stack. */
> +	if (ack.mask & COREDUMP_RECORDS) {
> +		cprm->record_hdr = kmalloc_obj(*cprm->record_hdr);
> +		if (!cprm->record_hdr)
> +			return false;
> +	}
> +
>   	cprm->mask = ack.mask;
>   	return coredump_sock_mark(cprm->file, COREDUMP_MARK_REQACK);
>   }
> @@ -1046,7 +1062,6 @@ static bool coredump_pipe(struct core_name *cn, struct coredump_params *cprm,
>   static bool coredump_write(struct coredump_params *cprm,
>   			   const struct linux_binfmt *binfmt)
>   {
> -
>   	if (dump_interrupted()) {
>   		cprm->state |= COREDUMP_STATE_TRUNCATED;
>   		return true;
> @@ -1062,15 +1077,17 @@ static bool coredump_write(struct coredump_params *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
> +	 * file position. This prevents gdb from complaining about
>   	 * a truncated file if the last "write" to the file was
> -	 * dump_skip.
> +	 * dump_skip. A record stream relies on it too: the flush
> +	 * emits the records that cover a trailing hole.
>   	 */
>   	if (cprm->to_skip) {
>   		cprm->to_skip--;
>   		if (!dump_emit(cprm, "", 1))
>   			cprm->state |= COREDUMP_STATE_TRUNCATED;
>   	}
> +	dump_end_record(cprm);
>   	file_end_write(cprm->file);
>   	free_vma_snapshot(cprm);
>   	return true;
> @@ -1085,6 +1102,7 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
>   		atomic_dec(&core_pipe_count);
>   	}
>   	kfree(cn->corename);
> +	kfree(cprm->record_hdr);
>   	coredump_finish(cprm->state);
>   }
>   
> @@ -1218,26 +1236,74 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
>    * do on a core-file: use only these functions to write out all the
>    * necessary info.
>    */
> -/* One write, never more than a page. See __dump_emit(). */
> -static bool dump_emit_chunk(struct coredump_params *cprm, const void *addr,
> -			    int nr)
> +static bool dump_records(const struct coredump_params *cprm)
> +{
> +	return cprm->mask & COREDUMP_RECORDS;
> +}
> +
> +/* Describe the next @len bytes of the coredump. Returns the header size. */
> +static size_t dump_record_init(struct coredump_params *cprm,
> +			       enum coredump_record_type type, u64 flags,
> +			       u64 len)
> +{
> +	if (!dump_records(cprm))
> +		return 0;
> +
> +	*cprm->record_hdr = (struct coredump_record_header) {
> +		.size	= sizeof(*cprm->record_hdr),
> +		.type	= type,
> +		.flags	= flags,
> +		.offset	= cprm->pos,
> +		.len	= len,
> +	};
> +
> +	return sizeof(*cprm->record_hdr);
> +}
> +
> +/* Write @iter whole or fail. @len is what it advances the coredump by. */
> +static bool dump_write_iter(struct coredump_params *cprm, struct iov_iter *iter,
> +			    size_t len)
>   {
>   	struct file *file = cprm->file;
> +	size_t count = iov_iter_count(iter);
>   	loff_t pos = file->f_pos;
>   	ssize_t n;
>   
> -	if (dump_interrupted())
> +	n = __kernel_write_iter(file, iter, &pos);
> +	if (n != (ssize_t)count)
>   		return false;
> +	file->f_pos = pos;
> +	cprm->written += count;
> +	cprm->pos += len;
> +
> +	return true;
> +}
> +
> +/* One record, never more than a page. See __dump_emit(). */
> +static bool dump_emit_chunk(struct coredump_params *cprm, const void *addr,
> +			    int nr)
> +{
> +	struct kvec kvec[2];
> +	struct iov_iter iter;
> +	unsigned int nseg = 0;
> +	size_t hdrlen;
>   
> -	n = __kernel_write(file, addr, nr, &pos);
> -	if (n != nr)
> +	if (dump_interrupted())
>   		return false;
>   
> -	file->f_pos = pos;
> -	cprm->written += n;
> -	cprm->pos += n;
> +	hdrlen = dump_record_init(cprm, COREDUMP_RECORD_DATA, 0, nr);
> +	if (hdrlen) {
> +		kvec[nseg].iov_base = cprm->record_hdr;
> +		kvec[nseg].iov_len = hdrlen;
> +		nseg++;
> +	}
> +	kvec[nseg].iov_base = (void *)addr;
> +	kvec[nseg].iov_len = nr;
> +	nseg++;
>   
> -	return true;
> +	iov_iter_kvec(&iter, ITER_SOURCE, kvec, nseg, hdrlen + nr);
> +
> +	return dump_write_iter(cprm, &iter, nr);
>   }
>   
>   static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
> @@ -1258,6 +1324,34 @@ static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
>   	return true;
>   }
>   
> +/* Send a record that stands on its own: a header and nothing else. */
> +static bool dump_emit_record(struct coredump_params *cprm,
> +			     enum coredump_record_type type, u64 flags, u64 len)
> +{
> +	struct kvec kvec;
> +	struct iov_iter iter;
> +	size_t hdrlen;
> +
> +	hdrlen = dump_record_init(cprm, type, flags, len);
> +	if (!hdrlen)
> +		return false;
> +
> +	kvec.iov_base = cprm->record_hdr;
> +	kvec.iov_len = hdrlen;
> +	iov_iter_kvec(&iter, ITER_SOURCE, &kvec, 1, hdrlen);
> +
> +	return dump_write_iter(cprm, &iter, len);
> +}
> +
> +/* Close the record stream. Only a whole coredump gets an end record. */
> +static void dump_end_record(struct coredump_params *cprm)
> +{
> +	if (cprm->state & COREDUMP_STATE_TRUNCATED)
> +		return;
> +
> +	dump_emit_record(cprm, COREDUMP_RECORD_END, 0, 0);
> +}
> +
>   static bool __dump_skip(struct coredump_params *cprm, size_t nr)
>   {
>   	static char zeroes[PAGE_SIZE];
> @@ -1318,11 +1412,10 @@ EXPORT_SYMBOL(dump_skip);
>   #ifdef CONFIG_ELF_CORE
>   static bool dump_emit_page(struct coredump_params *cprm, struct page *page)
>   {
> -	struct bio_vec bvec;
> +	struct bio_vec bvec[2];
>   	struct iov_iter iter;
> -	struct file *file = cprm->file;
> -	loff_t pos;
> -	ssize_t n;
> +	unsigned int nseg = 0;
> +	size_t hdrlen;
>   
>   	if (!page)
>   		return false;
> @@ -1333,17 +1426,16 @@ static bool dump_emit_page(struct coredump_params *cprm, struct page *page)
>   		return false;
>   	if (dump_interrupted())
>   		return false;
> -	pos = file->f_pos;
> -	bvec_set_page(&bvec, page, PAGE_SIZE, 0);
> -	iov_iter_bvec(&iter, ITER_SOURCE, &bvec, 1, PAGE_SIZE);
> -	n = __kernel_write_iter(cprm->file, &iter, &pos);
> -	if (n != PAGE_SIZE)
> -		return false;
> -	file->f_pos = pos;
> -	cprm->written += PAGE_SIZE;
> -	cprm->pos += PAGE_SIZE;
>   
> -	return true;
> +	/* Hand the record header to the same write as the page it describes. */
> +	hdrlen = dump_record_init(cprm, COREDUMP_RECORD_DATA, 0, PAGE_SIZE);
> +	if (hdrlen)
> +		bvec_set_virt(&bvec[nseg++], cprm->record_hdr, hdrlen);
> +	bvec_set_page(&bvec[nseg++], page, PAGE_SIZE, 0);
> +
> +	iov_iter_bvec(&iter, ITER_SOURCE, bvec, nseg, hdrlen + PAGE_SIZE);
> +
> +	return dump_write_iter(cprm, &iter, PAGE_SIZE);
>   }
>   
>   /*
> diff --git a/include/linux/coredump.h b/include/linux/coredump.h
> index 709388dd5659..b252bb2843b3 100644
> --- a/include/linux/coredump.h
> +++ b/include/linux/coredump.h
> @@ -6,6 +6,7 @@
>   #include <linux/mm.h>
>   #include <linux/fs.h>
>   #include <linux/sched/coredump.h>
> +#include <uapi/linux/coredump.h>
>   #include <asm/siginfo.h>
>   
>   #ifdef CONFIG_COREDUMP
> @@ -40,7 +41,11 @@ struct coredump_params {
>   	u64 mask;
>   	/* COREDUMP_STATE_* raised while the coredump is written. */
>   	enum coredump_state state;
> +	/* Record header scratch, NULL unless the coredump is a record stream. */
> +	struct coredump_record_header *record_hdr;
> +	/* Bytes handed to the file, record headers included. */
>   	loff_t written;
> +	/* Offset in the coredump, record headers excluded. */
>   	loff_t pos;
>   	loff_t to_skip;
>   	int vma_count;
> diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
> index 570fc2e005c2..1c8658f35735 100644
> --- a/tools/testing/selftests/coredump/coredump_test_helpers.c
> +++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
> @@ -275,7 +275,7 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
>   /* Every option the kernel is expected to advertise in coredump_req->mask. */
>   #define TEST_REQ_MASK_ALL					\
>   	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
> -	 COREDUMP_REJECT | COREDUMP_WAIT)
> +	 COREDUMP_REJECT | COREDUMP_WAIT | COREDUMP_RECORDS)
>   
>   bool check_coredump_req(const struct coredump_req *req)
>   {
>
-- 
Thanks and Regards
R.Nageswara Sastry


  reply	other threads:[~2026-09-09 16:05 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
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 [this message]
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=c578bc04-3128-426a-bedb-b53155510f55@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;
as well as URLs for NNTP newsgroup(s).