All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Yonghong Song <yonghong.song@linux.dev>
Cc: Jiri Olsa <olsajiri@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Ilya Leoshkevich <iii@linux.ibm.com>,
	bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>
Subject: Re: [PATCHv3 bpf 2/2] selftests/bpf: Add test for early update in prog_array_map_poke_run
Date: Tue, 5 Dec 2023 22:57:33 +0100	[thread overview]
Message-ID: <ZW-czRcdvQL7A1e_@krava> (raw)
In-Reply-To: <f18d75bc-1d1c-4391-b006-308568de10bf@linux.dev>

On Tue, Dec 05, 2023 at 08:00:48AM -0800, Yonghong Song wrote:

SNIP

> > > > +void test_tailcall_poke(void)
> > > > +{
> > > > +	struct tailcall_poke *call, *test;
> > > > +	int err, cnt = 10;
> > > > +	pthread_t thread;
> > > > +
> > > > +	unlink(JMP_TABLE);
> > > > +
> > > > +	call = tailcall_poke__open_and_load();
> > > > +	if (!ASSERT_OK_PTR(call, "tailcall_poke__open"))
> > > > +		return;
> > > > +
> > > > +	err = bpf_map__pin(call->maps.jmp_table, JMP_TABLE);
> > > > +	if (!ASSERT_OK(err, "bpf_map__pin"))
> > > > +		goto out;
> > > Just curious. What is the reason having bpf_map__pin() here
> > > and below? I tried and it looks like removing bpf_map__pin()
> > > and below bpf_map__set_pin_path() will make reproducing
> > > the failure hard/impossible.
> > yes, it's there to share the jmp_table map between the two
> > skeleton instances, so the update thread changes the same
> > jmp_table map that's used in the skeleton we load in the
> > while loop below
> 
> This does make sense.
> 
> > 
> > I'll add some comments to the test
> 
> Thanks for explanation. Some comments are definitely helpful!

np, also looks like I should move this to prog_tests/tailcalls.c,
will send new version with that

thanks,
jirka


> 
> > 
> > jirka
> > 
> > > > +
> > > > +	err = pthread_create(&thread, NULL, update, call);
> > > > +	if (!ASSERT_OK(err, "new toggler"))
> > > > +		goto out;
> > > > +
> > > > +	while (cnt--) {
> > > > +		test = tailcall_poke__open();
> > > > +		if (!ASSERT_OK_PTR(test, "tailcall_poke__open"))
> > > > +			break;
> > > > +
> > > > +		err = bpf_map__set_pin_path(test->maps.jmp_table, JMP_TABLE);
> > > > +		if (!ASSERT_OK(err, "bpf_map__pin")) {
> > > > +			tailcall_poke__destroy(test);
> > > > +			break;
> > > > +		}
> > > > +
> > > > +		bpf_program__set_autoload(test->progs.test, true);
> > > > +		bpf_program__set_autoload(test->progs.call1, false);
> > > > +		bpf_program__set_autoload(test->progs.call2, false);
> > > > +
> > > > +		err = tailcall_poke__load(test);
> > > > +		tailcall_poke__destroy(test);
> > > > +		if (!ASSERT_OK(err, "tailcall_poke__load"))
> > > > +			break;
> > > > +	}
> > > > +
> > > > +	thread_exit = 1;
> > > > +	ASSERT_OK(pthread_join(thread, NULL), "pthread_join");
> > > > +
> > > > +out:
> > > > +	bpf_map__unpin(call->maps.jmp_table, JMP_TABLE);
> > > > +	tailcall_poke__destroy(call);
> > > > +}
> > SNIP

      reply	other threads:[~2023-12-05 21:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-03 20:48 [PATCHv3 bpf 0/2] bpf: Fix map poke update Jiri Olsa
2023-12-03 20:48 ` [PATCHv3 bpf 1/2] bpf: Fix prog_array_map_poke_run " Jiri Olsa
2023-12-05  4:52   ` Yonghong Song
2023-12-05  6:56   ` kernel test robot
2023-12-05  7:17   ` kernel test robot
2023-12-03 20:48 ` [PATCHv3 bpf 2/2] selftests/bpf: Add test for early update in prog_array_map_poke_run Jiri Olsa
2023-12-05  5:16   ` Yonghong Song
2023-12-05  8:43     ` Jiri Olsa
2023-12-05 16:00       ` Yonghong Song
2023-12-05 21:57         ` Jiri Olsa [this message]

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=ZW-czRcdvQL7A1e_@krava \
    --to=olsajiri@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=iii@linux.ibm.com \
    --cc=john.fastabend@gmail.com \
    --cc=kafai@fb.com \
    --cc=kpsingh@chromium.org \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=yhs@fb.com \
    --cc=yonghong.song@linux.dev \
    /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.