Re: [PATCH v2 17/22] coredump: describe the holes when COREDUMP_SPARSE is negotiated

[R Nageswara Sastry <rnsastry@linux.ibm.com>]

On 20.08.2026 4:39 AM, Christian Brauner wrote:
> Make use of COREDUMP_SPARSE. Refuse it without COREDUMP_RECORDS.
>
> Actual holes are sent as a record with length indicating how much zero
> data there was.
>
> coredump_write() flushes a trailing hole if the coredump is done.
> Instead of writing the actual byte for pipes and sockets, collapse it.
> This stops wasting a header with coredump records for a single byte. So
> we now only write it when the coredump can be seeked. TL;DR a trailing
> hole is a zero record like any other and the records still cover the
> whole coredump.
>
> 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                                      | 41 +++++++++++++++++-----
>   .../selftests/coredump/coredump_test_helpers.c     |  3 +-
>   2 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/fs/coredump.c b/fs/coredump.c
> index b1679930094c..7b568d25887c 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -68,6 +68,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);
> +static bool dump_flush_skip(struct coredump_params *cprm);
>   
>   #define CORE_FILE_NOTE_SIZE_DEFAULT (4*1024*1024)
>   /* Define a reasonable max cap */
> @@ -806,7 +807,7 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
>   		.size		= sizeof(struct coredump_req),
>   		.mask		= COREDUMP_KERNEL | COREDUMP_USERSPACE |
>   				  COREDUMP_REJECT | COREDUMP_WAIT |
> -				  COREDUMP_RECORDS,
> +				  COREDUMP_RECORDS | COREDUMP_SPARSE,
>   		.size_ack	= sizeof(struct coredump_ack),
>   	};
>   	struct coredump_ack ack = {};
> @@ -866,6 +867,12 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
>   		return false;
>   	}
>   
> +	/* Zero records only exist inside a record stream. */
> +	if ((ack.mask & COREDUMP_SPARSE) && !(ack.mask & COREDUMP_RECORDS)) {
> +		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);
> @@ -1076,15 +1083,21 @@ static bool coredump_write(struct coredump_params *cprm,
>   	if (!binfmt->core_dump(cprm))
>   		cprm->state |= COREDUMP_STATE_TRUNCATED;
>   	/*
> -	 * Ensures that file size is big enough to contain the current
> -	 * file position. This prevents gdb from complaining about
> -	 * a truncated file if the last "write" to the file was
> -	 * dump_skip. A record stream relies on it too: the flush
> -	 * emits the records that cover a trailing hole.
> +	 * A trailing hole still has to land in the coredump. Seeking over
> +	 * it doesn't grow the file, so the last byte of it is written
> +	 * instead and gdb doesn't see a truncated file. Everything else
> +	 * puts the hole on the wire as it flushes it.
>   	 */
>   	if (cprm->to_skip) {
> -		cprm->to_skip--;
> -		if (!dump_emit(cprm, "", 1))
> +		bool flushed;
> +
> +		if (cprm->file->f_mode & FMODE_LSEEK) {
> +			cprm->to_skip--;
> +			flushed = dump_emit(cprm, "", 1);
> +		} else {
> +			flushed = dump_flush_skip(cprm);
> +		}
> +		if (!flushed)
>   			cprm->state |= COREDUMP_STATE_TRUNCATED;
>   	}
>   	dump_end_record(cprm);
> @@ -1241,6 +1254,11 @@ static bool dump_records(const struct coredump_params *cprm)
>   	return cprm->mask & COREDUMP_RECORDS;
>   }
>   
> +static bool dump_sparse(const struct coredump_params *cprm)
> +{
> +	return cprm->mask & COREDUMP_SPARSE;
> +}
> +
>   /* 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,
> @@ -1357,6 +1375,13 @@ static bool __dump_skip(struct coredump_params *cprm, size_t nr)
>   	static char zeroes[PAGE_SIZE];
>   	struct file *file = cprm->file;
>   
> +	if (dump_sparse(cprm)) {
> +		/* Hand the server the length of the hole instead of the hole itself. */
> +		if (dump_interrupted())
> +			return false;
> +		return dump_emit_record(cprm, COREDUMP_RECORD_ZERO, 0, nr);
> +	}
> +
>   	if (file->f_mode & FMODE_LSEEK) {
>   		if (dump_interrupted() || vfs_llseek(file, nr, SEEK_CUR) < 0)
>   			return false;
> diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
> index 1c8658f35735..a5b9cde47239 100644
> --- a/tools/testing/selftests/coredump/coredump_test_helpers.c
> +++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
> @@ -275,7 +275,8 @@ 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_RECORDS)
> +	 COREDUMP_REJECT | COREDUMP_WAIT |			\
> +	 COREDUMP_RECORDS | COREDUMP_SPARSE)
>   
>   bool check_coredump_req(const struct coredump_req *req)
>   {
>
-- 
Thanks and Regards
R.Nageswara Sastry