Thread (1 message) 1 message, 1 author, 2018-01-17
DORMANTno replies

[PATCH] kernel:bpf Remove structure passing and assignment to save stack and no coping structures

From: Karim Eshapa <hidden>
Date: 2018-01-17 21:32:26
Also in: lkml

On Sun, Jan 14, 2018 at 01:18:35PM +0200, Karim Eshapa wrote:
quoted
quoted
quoted
Use pointers to structure as arguments to function instead of coping
structures and less stack size. Also transfer TNUM(_v, _m) to
tnum.h file to be used in differnet files for creating anonymous structures
statically.

Signed-off-by: Karim Eshapa <redacted>
...
quoted
quoted
+/* Statically tnum constant */
+#define TNUM(_v, _m) (struct tnum){.value = _v, .mask = _m}
 /* Represent a known constant as a tnum. */
 struct tnum tnum_const(u64 value);
 /* A completely unknown value */
@@ -26,7 +28,7 @@ struct tnum tnum_lshift(struct tnum a, u8 shift);
 /* Shift a tnum right (by a fixed shift) */
 struct tnum tnum_rshift(struct tnum a, u8 shift);
 /* Add two tnums, return @a + @b */
-struct tnum tnum_add(struct tnum a, struct tnum b);
+void tnum_add(struct tnum *res, struct tnum *a, struct tnum *b);
...
quoted
quoted
-     reg_off = tnum_add(reg->var_off, tnum_const(ip_align + reg->off + off));
+     tnum_add(&reg_off, &reg->var_off, &TNUM(ip_align + reg->off + off, 0));
      if (!tnum_is_aligned(reg_off, size)) {
              char tn_buf[48];
@@ -1023,8 +1023,7 @@ static int check_generic_ptr_alignment(struct bpf_verifier_env *env,
      /* Byte size accesses are always allowed. */
      if (!strict || size == 1)
              return 0;
-
-     reg_off = tnum_add(reg->var_off, tnum_const(reg->off + off));
+     tnum_add(&reg_off, &reg->var_off, &TNUM(reg->off + off, 0));
...
quoted
-             dst_reg->var_off = tnum_add(ptr_reg->var_off, off_reg->var_off);
quoted
+             tnum_add(&dst_reg->var_off, &ptr_reg->var_off,
+                     &off_reg->var_off);
quoted
Is it gnu or intel style of argumnets ? where is src or dest ?
Can the same pointer be used as src and as dst ? etc, etc
I don't think it saves stack either.
I'd rather leave things as-is.
It's not specific style but it's recommended when passing structure specially if
the structures have large sizes.
and (dest, src0, src1) respectively.Although tnum structure isn't large but it saves
quoted
stack,we have 2 structure passed before calling and 1 returned to receive the return value.
1. your patch has compile time warnings
2. it doesn't reduce stack size.
  For two functions that use tnum_add:
  adjust_ptr_min_max_vals() before and after has exactly the same.
  check_ptr_alignment() after your patch _increased_ stack size.
3. text of verifier.o shrank 133 bytes while tnum.o increased 198
Please do your homework next time.
tnum code will stay as-is.
Thanks so much for your response,if there is any recommended tools
to test how your patch affect memory, performance and what's
going on because all accepted patches I sumbited was so trivial 
I'll be so appreciated.

Karim, 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help