Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..4f071943ffb0 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,6 +140,7 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp); + fsync(fd);
if (!ASSERT_OK(ferror(fp), "prepare tmp")) goto out_fclose;
On Tue, Oct 14, 2025 at 04:03:23PM +0800, Xing Guo wrote:
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com
tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..4f071943ffb0 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,6 +140,7 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp);
- fsync(fd);
could we just close the fp stream instead flushing it twice?
maybe something like below, but not sure ferror will work after the fclose call
jirka
--- diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..5a4c1bca2a1e 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -139,10 +139,10 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest # subtest duplicate\n"); fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); - fflush(fp); + fclose(fp);
if (!ASSERT_OK(ferror(fp), "prepare tmp")) - goto out_fclose; + goto out_remove;
init_test_filter_set(&set);
@@ -160,8 +160,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);
-out_fclose: - fclose(fp); out_remove: remove(tmpfile); }
On Tue, Oct 14, 2025 at 5:39 AM Jiri Olsa olsajiri@gmail.com wrote:
On Tue, Oct 14, 2025 at 04:03:23PM +0800, Xing Guo wrote:
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com
tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..4f071943ffb0 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,6 +140,7 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp);
fsync(fd);could we just close the fp stream instead flushing it twice?
maybe something like below, but not sure ferror will work after the fclose call
jirka
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..5a4c1bca2a1e 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -139,10 +139,10 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest # subtest duplicate\n"); fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline");
fflush(fp);
fclose(fp);
we should probably fclose() after ferror().
but the original fix works, though I think we should do fsync(fp) instead to say within FILE-based APIs.
pw-bot: cr
if (!ASSERT_OK(ferror(fp), "prepare tmp"))
goto out_fclose;
goto out_remove; init_test_filter_set(&set);@@ -160,8 +160,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);-out_fclose:
fclose(fp);out_remove: remove(tmpfile); }
Thanks for reviewing! Here's the revised patch.
--- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..0f99f06116ea 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,9 +140,11 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp); - - if (!ASSERT_OK(ferror(fp), "prepare tmp")) - goto out_fclose; + if (!ASSERT_OK(ferror(fp), "prepare tmp")) { + fclose(fp); + goto out_remove; + } + fclose(fp);
init_test_filter_set(&set);
@@ -160,8 +162,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);
-out_fclose: - fclose(fp); out_remove: remove(tmpfile); }
On Tue, Oct 14, 2025 at 7:43 PM Xing Guo higuoxing@gmail.com wrote:
Thanks for reviewing! Here's the revised patch.
Please send it as a proper patch with SOB, etc.
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..0f99f06116ea 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,9 +140,11 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp); - - if (!ASSERT_OK(ferror(fp), "prepare tmp")) - goto out_fclose; + if (!ASSERT_OK(ferror(fp), "prepare tmp")) { + fclose(fp); + goto out_remove; + } + fclose(fp);
init_test_filter_set(&set);
@@ -160,8 +162,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);
-out_fclose: - fclose(fp); out_remove: remove(tmpfile); }
Hello:
This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko andrii@kernel.org:
On Wed, 15 Oct 2025 10:50:49 +0800 you wrote:
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
[...]
Here is the summary with links: - [v2] selftests: arg_parsing: Ensure data is flushed to disk before reading. https://git.kernel.org/bpf/bpf-next/c/27aab47b347e
You are awesome, thank you!
On Tue, Oct 14, 2025 at 7:50 PM Xing Guo higuoxing@gmail.com wrote:
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com
tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
I accidentally applied it to bpf-next tree and had to drop it from there.
It actually doesn't apply to bpf tree cleanly, so please rebase and resend.
But first, did you validate that fclose() actually fixes the issue for you? Because I don't think fclose() will call fsync(), will it? fclose() will basically do the same thing as should be done with fflush() anyways, so I have my doubts.
Furthermore, your commit message doesn't correspond to your patch. There is no fsync() call here, please fix that as well.
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..0f99f06116ea 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,9 +140,11 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp);
if (!ASSERT_OK(ferror(fp), "prepare tmp"))goto out_fclose;
if (!ASSERT_OK(ferror(fp), "prepare tmp")) {fclose(fp);goto out_remove;}fclose(fp); init_test_filter_set(&set);@@ -160,8 +162,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);-out_fclose:
fclose(fp);out_remove: remove(tmpfile); } -- 2.51.0
test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call. --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index fbf0d9c2f58b..e27d66b75fb1 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -144,6 +144,9 @@ static void test_parse_test_list_file(void) if (!ASSERT_OK(ferror(fp), "prepare tmp")) goto out_fclose;
+ if (!ASSERT_OK(fsync(fileno(fp)), "fsync tmp")) + goto out_fclose; + init_test_filter_set(&set);
if (!ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file"))
On Thu, Oct 16, 2025 at 12:20 AM Andrii Nakryiko andrii.nakryiko@gmail.com wrote:
On Tue, Oct 14, 2025 at 7:50 PM Xing Guo higuoxing@gmail.com wrote:
Recently, I noticed a selftest failure in my local environment. The test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com
tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
I accidentally applied it to bpf-next tree and had to drop it from there.
It actually doesn't apply to bpf tree cleanly, so please rebase and resend.
But first, did you validate that fclose() actually fixes the issue for you? Because I don't think fclose() will call fsync(), will it? fclose() will basically do the same thing as should be done with fflush() anyways, so I have my doubts.
Thanks for pointing this out. Adding fclose() resolves the issue on my laptop too. I prefer switching back to using fsync() according to your comment.
Best Regards, Xing.
Furthermore, your commit message doesn't correspond to your patch. There is no fsync() call here, please fix that as well.
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index bb143de68875..0f99f06116ea 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -140,9 +140,11 @@ static void test_parse_test_list_file(void) fprintf(fp, "testA/subtest2\n"); fprintf(fp, "testC_no_eof_newline"); fflush(fp);
if (!ASSERT_OK(ferror(fp), "prepare tmp"))goto out_fclose;
if (!ASSERT_OK(ferror(fp), "prepare tmp")) {fclose(fp);goto out_remove;}fclose(fp); init_test_filter_set(&set);@@ -160,8 +162,6 @@ static void test_parse_test_list_file(void)
free_test_filter_set(&set);-out_fclose:
fclose(fp);out_remove: remove(tmpfile); } -- 2.51.0
test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call. --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index fbf0d9c2f58b..e27d66b75fb1 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -144,6 +144,9 @@ static void test_parse_test_list_file(void) if (!ASSERT_OK(ferror(fp), "prepare tmp")) goto out_fclose;
+ if (!ASSERT_OK(fsync(fileno(fp)), "fsync tmp")) + goto out_fclose; + init_test_filter_set(&set);
if (!ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file"))
test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Signed-off-by: Xing Guo higuoxing@gmail.com --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index fbf0d9c2f58b..e27d66b75fb1 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -144,6 +144,9 @@ static void test_parse_test_list_file(void) if (!ASSERT_OK(ferror(fp), "prepare tmp")) goto out_fclose;
+ if (!ASSERT_OK(fsync(fileno(fp)), "fsync tmp")) + goto out_fclose; + init_test_filter_set(&set);
if (!ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file"))
test_parse_test_list_file writes some data to /tmp/bpf_arg_parsing_test.XXXXXX and parse_test_list_file() will read the data back. However, after writing data to that file, we forget to call fsync() and it's causing testing failure in my laptop. This patch helps fix it by adding the missing fsync() call.
Fixes: 64276f01dce8 ("selftests/bpf: Test_progs can read test lists from file") Signed-off-by: Xing Guo higuoxing@gmail.com --- tools/testing/selftests/bpf/prog_tests/arg_parsing.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c index fbf0d9c2f58b..e27d66b75fb1 100644 --- a/tools/testing/selftests/bpf/prog_tests/arg_parsing.c +++ b/tools/testing/selftests/bpf/prog_tests/arg_parsing.c @@ -144,6 +144,9 @@ static void test_parse_test_list_file(void) if (!ASSERT_OK(ferror(fp), "prepare tmp")) goto out_fclose;
+ if (!ASSERT_OK(fsync(fileno(fp)), "fsync tmp")) + goto out_fclose; + init_test_filter_set(&set);
if (!ASSERT_OK(parse_test_list_file(tmpfile, &set, true), "parse file"))
linux-kselftest-mirror@lists.linaro.org
-
Alexei Starovoitov -
Andrii Nakryiko -
Jiri Olsa -
patchwork-bot+netdevbpf＠kernel.org -
Xing Guo