On Tue, Jun 13, 2023 at 04:09:21PM +0800, Hou Tao wrote:
> +
> +static void htab_mem_notify_wait_producer(pthread_barrier_t *notify)
notify_wait and wait_notify names are confusing.
The first one is doing map_update and 2nd is map_delete, right?
Just call them such?
> +{
> + while (true) {
> + (void)syscall(__NR_getpgid);
> + /* Notify for start */
the comment is confusing too.
Maybe /* Notify map_deleter that map_updates are done */ ?
> + pthread_barrier_wait(notify);
> + /* Wait for completion */
and /* Wait for deletions to complete */ ?
> + pthread_barrier_wait(notify);
> + }
> +}
> +
> +static void htab_mem_wait_notify_producer(pthread_barrier_t *notify)
> +{
> + while (true) {
> + /* Wait for start */
> + pthread_barrier_wait(notify);
> + (void)syscall(__NR_getpgid);
> + /* Notify for completion */
similar.
> + pthread_barrier_wait(notify);
> + }
> +}
> +static int write_htab(unsigned int i, struct update_ctx *ctx, unsigned int flags)
> +{
> + if (ctx->from >= MAX_ENTRIES)
> + return 1;
It can never be hit, right?
Remove it then?
> +
> + bpf_map_update_elem(&htab, &ctx->from, zeroed_value, flags);
please add error check.
I think update/delete notification is correct, but it could be silently broken.
update(BPF_NOEXIST) could be returning error in one thread and
map_delete_elem could be failing too...
> + ctx->from += ctx->step;
> +
> + return 0;
> +}
> +
> +static int overwrite_htab(unsigned int i, struct update_ctx *ctx)
> +{
> + return write_htab(i, ctx, 0);
> +}
> +
> +static int newwrite_htab(unsigned int i, struct update_ctx *ctx)
> +{
> + return write_htab(i, ctx, BPF_NOEXIST);
> +}
> +
> +static int del_htab(unsigned int i, struct update_ctx *ctx)
> +{
> + if (ctx->from >= MAX_ENTRIES)
> + return 1;
delete?
> +
> + bpf_map_delete_elem(&htab, &ctx->from);
and here.
> + ctx->from += ctx->step;
> +
> + return 0;
> +}
> +
> +SEC("?tp/syscalls/sys_enter_getpgid")
> +int overwrite(void *ctx)
> +{
> + struct update_ctx update;
> +
> + update.from = bpf_get_smp_processor_id();
> + update.step = nr_thread;
> + bpf_loop(64, overwrite_htab, &update, 0);
> + __sync_fetch_and_add(&op_cnt, 1);
> + return 0;
> +}
> +
> +SEC("?tp/syscalls/sys_enter_getpgid")
> +int batch_add_batch_del(void *ctx)
> +{
> + struct update_ctx update;
> +
> + update.from = bpf_get_smp_processor_id();
> + update.step = nr_thread;
> + bpf_loop(64, overwrite_htab, &update, 0);
> +
> + update.from = bpf_get_smp_processor_id();
> + bpf_loop(64, del_htab, &update, 0);
> +
> + __sync_fetch_and_add(&op_cnt, 2);
> + return 0;
> +}
> +
> +SEC("?tp/syscalls/sys_enter_getpgid")
> +int add_del_on_diff_cpu(void *ctx)
> +{
> + struct update_ctx update;
> + unsigned int from;
> +
> + from = bpf_get_smp_processor_id();
> + update.from = from / 2;
why extra 'from' variable? Just combine above two lines.
> + update.step = nr_thread / 2;
> +
> + if (from & 1)
> + bpf_loop(64, newwrite_htab, &update, 0);
> + else
> + bpf_loop(64, del_htab, &update, 0);
I think it's cleaner to split this into two bpf programs.
Do update(NOEXIST) in one triggered by sys_enter_getpgid
and do delete_elem() in another triggered by a different syscall.
> +
> + __sync_fetch_and_add(&op_cnt, 1);
> + return 0;
> +}
> --
> 2.29.2
>
