Skip to content

Commit

Permalink
bpf: make list_for_each_entry portable
Browse files Browse the repository at this point in the history
[Changes from V1:
- The __compat_break has been abandoned in favor of
  a more readable can_loop macro that can be used anywhere, including
  loop conditions.]

The macro list_for_each_entry is defined in bpf_arena_list.h as
follows:

  #define list_for_each_entry(pos, head, member)				\
	for (void * ___tmp = (pos = list_entry_safe((head)->first,		\
						    typeof(*(pos)), member),	\
			      (void *)0);					\
	     pos && ({ ___tmp = (void *)pos->member.next; 1; });		\
	     cond_break,							\
	     pos = list_entry_safe((void __arena *)___tmp, typeof(*(pos)), member))

The macro cond_break, in turn, expands to a statement expression that
contains a `break' statement.  Compound statement expressions, and the
subsequent ability of placing statements in the header of a `for'
loop, are GNU extensions.

Unfortunately, clang implements this GNU extension differently than
GCC:

- In GCC the `break' statement is bound to the containing "breakable"
  context in which the defining `for' appears.  If there is no such
  context, GCC emits a warning: break statement without enclosing `for'
  o `switch' statement.

- In clang the `break' statement is bound to the defining `for'.  If
  the defining `for' is itself inside some breakable construct, then
  clang emits a -Wgcc-compat warning.

This patch adds a new macro can_loop to bpf_experimental, that
implements the same logic than cond_break but evaluates to a boolean
expression.  The patch also changes all the current instances of usage
of cond_break withing the header of loop accordingly.

Tested in bpf-next master.
No regressions.

Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
Cc: david.faust@oracle.com
Cc: cupertino.miranda@oracle.com
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Link: https://lore.kernel.org/r/20240511212243.23477-1-jose.marchesi@oracle.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
jemarch authored and Alexei Starovoitov committed May 13, 2024
1 parent 6a2f786 commit ba39486
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 10 deletions.
4 changes: 2 additions & 2 deletions tools/testing/selftests/bpf/bpf_arena_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ static inline void *bpf_iter_num_new(struct bpf_iter_num *it, int i, int j) { re
static inline void bpf_iter_num_destroy(struct bpf_iter_num *it) {}
static inline bool bpf_iter_num_next(struct bpf_iter_num *it) { return true; }
#define cond_break ({})
#define can_loop true
#endif

/* Safely walk link list elements. Deletion of elements is allowed. */
#define list_for_each_entry(pos, head, member) \
for (void * ___tmp = (pos = list_entry_safe((head)->first, \
typeof(*(pos)), member), \
(void *)0); \
pos && ({ ___tmp = (void *)pos->member.next; 1; }); \
cond_break, \
pos && ({ ___tmp = (void *)pos->member.next; 1; }) && can_loop; \
pos = list_entry_safe((void __arena *)___tmp, typeof(*(pos)), member))

static inline void list_add_head(arena_list_node_t *n, arena_list_head_t *h)
Expand Down
33 changes: 31 additions & 2 deletions tools/testing/selftests/bpf/bpf_experimental.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,19 +326,48 @@ l_true: \
})
#endif

/*
* Note that cond_break can only be portably used in the body of a breakable
* construct, whereas can_loop can be used anywhere.
*/
#ifdef __BPF_FEATURE_MAY_GOTO
#define can_loop \
({ __label__ l_break, l_continue; \
bool ret = true; \
asm volatile goto("may_goto %l[l_break]" \
:::: l_break); \
goto l_continue; \
l_break: ret = false; \
l_continue:; \
ret; \
})

#define cond_break \
({ __label__ l_break, l_continue; \
asm volatile goto("may_goto %l[l_break]" \
asm volatile goto("may_goto %l[l_break]" \
:::: l_break); \
goto l_continue; \
l_break: break; \
l_continue:; \
})
#else
#define can_loop \
({ __label__ l_break, l_continue; \
bool ret = true; \
asm volatile goto("1:.byte 0xe5; \
.byte 0; \
.long ((%l[l_break] - 1b - 8) / 8) & 0xffff; \
.short 0" \
:::: l_break); \
goto l_continue; \
l_break: ret = false; \
l_continue:; \
ret; \
})

#define cond_break \
({ __label__ l_break, l_continue; \
asm volatile goto("1:.byte 0xe5; \
asm volatile goto("1:.byte 0xe5; \
.byte 0; \
.long ((%l[l_break] - 1b - 8) / 8) & 0xffff; \
.short 0" \
Expand Down
2 changes: 1 addition & 1 deletion tools/testing/selftests/bpf/progs/arena_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ int arena_list_add(void *ctx)

list_head = &global_head;

for (i = zero; i < cnt; cond_break, i++) {
for (i = zero; i < cnt && can_loop; i++) {
struct elem __arena *n = bpf_alloc(sizeof(*n));

test_val++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ int cond_break1(const void *ctx)
unsigned long i;
unsigned int sum = 0;

for (i = zero; i < ARR_SZ; cond_break, i++)
for (i = zero; i < ARR_SZ && can_loop; i++)
sum += i;
for (i = zero; i < ARR_SZ; i++) {
barrier_var(i);
Expand All @@ -336,20 +336,19 @@ int cond_break2(const void *ctx)
int i, j;
int sum = 0;

for (i = zero; i < 1000; cond_break, i++)
for (i = zero; i < 1000 && can_loop; i++)
for (j = zero; j < 1000; j++) {
sum += i + j;
cond_break;
}

}
return sum;
}

static __noinline int loop(void)
{
int i, sum = 0;

for (i = zero; i <= 1000000; i++, cond_break)
for (i = zero; i <= 1000000 && can_loop; i++)
sum += i;

return sum;
Expand Down

0 comments on commit ba39486

Please sign in to comment.