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 05/22] selftests/coredump: collapse the expected request check into the helper
Date: Wed, 9 Sep 2026 21:24:44 +0530 [thread overview]
Message-ID: <233bec97-49d2-4871-b9a2-150586aead35@linux.ibm.com> (raw)
In-Reply-To: <20260820-work-coredump-sparse-v2-5-ba32dd718c51@kernel.org>
On 20.08.2026 4:39 AM, Christian Brauner wrote:
> All nine callers spell the expected flags. So every new feature bit the
> kernel learns has to be cargo culted.
>
> The callers also all pass COREDUMP_ACK_SIZE_VER0 as the minimum request
> size although what is being validated is coredump_req->size. And
> read_coredump_req() makes the same mixup twice more.
>
> Clean this all up.
>
> 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
> ---
> .../coredump/coredump_socket_protocol_test.c | 36 ++++++----------------
> tools/testing/selftests/coredump/coredump_test.h | 3 +-
> .../selftests/coredump/coredump_test_helpers.c | 35 +++++++++++++--------
> 3 files changed, 32 insertions(+), 42 deletions(-)
>
> diff --git a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
> index d9fa6239b5a9..60a357e628eb 100644
> --- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
> +++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
> @@ -151,9 +151,7 @@ TEST_F(coredump, socket_request_kernel)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_kernel: check_coredump_req failed\n");
> goto out;
> }
> @@ -301,9 +299,7 @@ TEST_F(coredump, socket_request_userspace)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_userspace: check_coredump_req failed\n");
> goto out;
> }
> @@ -441,9 +437,7 @@ TEST_F(coredump, socket_request_reject)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_reject: check_coredump_req failed\n");
> goto out;
> }
> @@ -581,9 +575,7 @@ TEST_F(coredump, socket_request_invalid_flag_combination)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_invalid_flag_combination: check_coredump_req failed\n");
> goto out;
> }
> @@ -702,9 +694,7 @@ TEST_F(coredump, socket_request_unknown_flag)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_unknown_flag: check_coredump_req failed\n");
> goto out;
> }
> @@ -822,9 +812,7 @@ TEST_F(coredump, socket_request_invalid_size_small)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_invalid_size_small: check_coredump_req failed\n");
> goto out;
> }
> @@ -944,9 +932,7 @@ TEST_F(coredump, socket_request_invalid_size_large)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_request_invalid_size_large: check_coredump_req failed\n");
> goto out;
> }
> @@ -1355,9 +1341,7 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps, 500)
> goto out;
> }
>
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "check_coredump_req failed for fd %d\n", fd_coredump);
> goto out;
> }
> @@ -1509,9 +1493,7 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps_epoll_workers, 500)
> fprintf(stderr, "socket_multiple_crashing_coredumps_epoll_workers: read_coredump_req failed\n");
> goto out;
> }
> - if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
> - COREDUMP_KERNEL | COREDUMP_USERSPACE |
> - COREDUMP_REJECT | COREDUMP_WAIT)) {
> + if (!check_coredump_req(&req)) {
> fprintf(stderr, "socket_multiple_crashing_coredumps_epoll_workers: check_coredump_req failed\n");
> goto out;
> }
> diff --git a/tools/testing/selftests/coredump/coredump_test.h b/tools/testing/selftests/coredump/coredump_test.h
> index ed47f01fa53c..a02809145e2d 100644
> --- a/tools/testing/selftests/coredump/coredump_test.h
> +++ b/tools/testing/selftests/coredump/coredump_test.h
> @@ -51,8 +51,7 @@ bool read_marker(int fd, enum coredump_mark mark);
> bool read_coredump_req(int fd, struct coredump_req *req);
> bool send_coredump_ack(int fd, const struct coredump_req *req,
> __u64 mask, size_t size_ack);
> -bool check_coredump_req(const struct coredump_req *req, size_t min_size,
> - __u64 required_mask);
> +bool check_coredump_req(const struct coredump_req *req);
> int open_coredump_tmpfile(int fd_tmpfs_detached);
> void process_coredump_worker(int fd_coredump, int fd_peer_pidfd, int fd_core_file);
>
> diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
> index 524fa5370593..306711e1b24d 100644
> --- a/tools/testing/selftests/coredump/coredump_test_helpers.c
> +++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
> @@ -200,7 +200,7 @@ bool read_marker(int fd, enum coredump_mark mark)
> bool read_coredump_req(int fd, struct coredump_req *req)
> {
> ssize_t ret;
> - size_t field_size, user_size, ack_size, kernel_size, remaining_size;
> + size_t field_size, user_size, known_size, kernel_size, remaining_size;
>
> memset(req, 0, sizeof(*req));
> field_size = sizeof(req->size);
> @@ -214,9 +214,9 @@ bool read_coredump_req(int fd, struct coredump_req *req)
> }
> kernel_size = req->size;
>
> - if (kernel_size < COREDUMP_ACK_SIZE_VER0) {
> + if (kernel_size < COREDUMP_REQ_SIZE_VER0) {
> fprintf(stderr, "read_coredump_req: kernel_size %zu < min %d\n",
> - kernel_size, COREDUMP_ACK_SIZE_VER0);
> + kernel_size, COREDUMP_REQ_SIZE_VER0);
> return false;
> }
> if (kernel_size >= PAGE_SIZE) {
> @@ -225,11 +225,11 @@ bool read_coredump_req(int fd, struct coredump_req *req)
> return false;
> }
>
> - /* Use the minimum of user and kernel size to read the full request. */
> + /* Consume as much of the request as we know about. */
> user_size = sizeof(struct coredump_req);
> - ack_size = user_size < kernel_size ? user_size : kernel_size;
> - ret = recv(fd, req, ack_size, MSG_WAITALL);
> - if (ret != ack_size)
> + known_size = user_size < kernel_size ? user_size : kernel_size;
> + ret = recv(fd, req, known_size, MSG_WAITALL);
> + if (ret != known_size)
> return false;
>
> fprintf(stderr, "Read coredump request with size %u and mask 0x%llx\n",
> @@ -287,15 +287,24 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
> return true;
> }
>
> -bool check_coredump_req(const struct coredump_req *req, size_t min_size,
> - __u64 required_mask)
> +/* 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)
> +
> +bool check_coredump_req(const struct coredump_req *req)
> {
> - if (req->size < min_size)
> - return false;
> - if ((req->mask & required_mask) != required_mask)
> + if (req->size < COREDUMP_REQ_SIZE_VER0) {
> + fprintf(stderr, "%s: size %u below minimum %d\n",
> + __func__, req->size, COREDUMP_REQ_SIZE_VER0);
> return false;
> - if (req->mask & ~required_mask)
> + }
> + if (req->mask != TEST_REQ_MASK_ALL) {
> + fprintf(stderr, "%s: mask 0x%llx, expected 0x%llx\n",
> + __func__, (unsigned long long)req->mask,
> + (unsigned long long)TEST_REQ_MASK_ALL);
> return false;
> + }
> return true;
> }
>
>
--
Thanks and Regards
R.Nageswara Sastry
next prev parent reply other threads:[~2026-09-09 15:55 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 [this message]
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
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=233bec97-49d2-4871-b9a2-150586aead35@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.