On Thu, 11 Jul 2024 at 01:28, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Tue, Jul 9, 2024 at 2:07 PM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote:
> > > + while (!*timer1_err && !*timer2_err)
> > > + bpf_prog_test_run_opts(prog_fd, &opts);
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +void test_timer_lockup(void)
> > > +{
> > > + struct timer_lockup *skel;
> > > + pthread_t thrds[2];
> > > + void *ret;
> > > +
> > > + skel = timer_lockup__open_and_load();
> > > + if (!ASSERT_OK_PTR(skel, "timer_lockup__open_and_load"))
> > > + return;
> > > +
> > > + int timer1_prog = bpf_program__fd(skel->progs.timer1_prog);
> > > + int timer2_prog = bpf_program__fd(skel->progs.timer2_prog);
>
> Pls don't declare inline. Some compiler might warn.
>
ack.
> > > +
> > > + timer1_err = &skel->bss->timer1_err;
> > > + timer2_err = &skel->bss->timer2_err;
> > > +
> > > + if (!ASSERT_OK(pthread_create(&thrds[0], NULL, timer_lockup_thread, &timer1_prog), "pthread_create thread1"))
>
> pls shorten the line.
>
ack.
> > > + return;
> > > + if (!ASSERT_OK(pthread_create(&thrds[1], NULL, timer_lockup_thread, &timer2_prog), "pthread_create thread2")) {
> > > + pthread_exit(&thrds[0]);
> > > + return;
> >
> > A goto out: timer_lockup___destroy(skel) is missing here and above
> > this. Will wait for a day or so before respinning.
>
> I was thinking to fix all these up while applying.
> So I fixed it, but then noticed that the new test is quite flaky.
> It seems it can get stuck in that while() loop forever.
> Pls investigate.
>
Will do.
> So I applied the first two patches only.
>
> Also pls fix:
> +static int timer_cb2(void *map, int *k, struct bpf_timer *timer)
> +{
> + int key = 0;
> +
> + timer = bpf_map_lookup_elem(&timer1_map, &key);
>
> 1. no need to do a lookup.
> 2. the 3rd arg is not a bpf_timer. It's a pointer to map value.
> So use
> timer_cb2(void *map, int *k, struct elem *v)
> then cast it to bpf_timer and use it w/o lookup.
2 makes sense, will fix. Lookup is still needed. We need the timer
from timer1_map, while the callback gets timer of timer2_map.
