include: add two exponential backoff macros
RFC PATCH 1/1 include: add two exponential backoff macros
--- include/tst_common.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
include/tst_common.h +47 -0
@@ -35,4 +35,51 @@#define LTP_ALIGN (x, a) __LTP_ALIGN_MASK(x, (typeof(x))(a) - 1)#define __LTP_ALIGN_MASK (x, mask) (((x) + (mask)) & ~(mask)) +/* +* Exponential backoff usleep for function repeat +* @FUNC: the function() which will be retried +* @ERET: an expected return value from the FUNC +*/ +#define TST_RETRY_FUNC (FUNC ,ERET ) \ + TST_RETRY_FUNC_WITH_EXPONENTIAL_BACKOFF(FUNC, ERET, 1) + +#define TST_RETRY_FUNC_WITH_EXPONENTIAL_BACKOFF (FUNC ,ERET ,MAX_DELAY ) \ +do { int delay = 1; \ + for (;;) { \ + typeof(FUNC) ret = FUNC; \ + if (ret == (typeof(FUNC))ERET) \ + break; \ + if (delay < MAX_DELAY * 1000000) { \ + usleep(delay); \ + delay *= 2; \ + } else { \ + tst_brk_(__FILE__, __LINE__, \ + TBROK | TERRNO, #FUNC " failed"); \ + } \ + } \ +} while(0) + +/* +* Exponential backoff usleep for wating a varible change +* @VAR: the variable will be changed in other place +* @EXP: an expected value which VAR should be equal to +*/ +#define TST_WAIT_ON_VAR (VAR ,EXP ) \ + TST_WAIT_WITH_EXPONENTIAL_BACKOFF(VAR, EXP, 1) + +#define TST_WAIT_WITH_EXPONENTIAL_BACKOFF (VAR ,EXP ,MAX_DELAY ) \ +do { int delay = 1; \ + for (;;) { \ + if (VAR == (typeof(VAR)) EXP) \ + break; \ + if (delay < MAX_DELAY * 1000000) { \ + usleep(delay); \ + delay *= 2; \ + } else { \ + tst_brk_(__FILE__, __LINE__, \ + TBROK | TERRNO, #VAR " is not expected"); \ + } \ + } \ +} while(0) +#endif /* TST_COMMON_H__ */
Signature · 1 line
1.9.3
RFC PATCH 1/1 include: add two exponential backoff macros
Hi!
include/tst_common.h line 46
Quoted 9 lines
include/tst_common.h +9
-0+/* +* Exponential backoff usleep for function repeat +* @FUNC: the function() which will be retried +* @ERET: an expected return value from the FUNC +*/ +#define TST_RETRY_FUNC (FUNC ,ERET ) \ + TST_RETRY_FUNC_WITH_EXPONENTIAL_BACKOFF(FUNC, ERET, 1) + +#define TST_RETRY_FUNC_WITH_EXPONENTIAL_BACKOFF (FUNC ,ERET ,MAX_DELAY ) \
That is quite a long name, I guess that we can shorten it a bit without loosing too much clarity. Something as TST_RETRY_FN_EXP_BACKOFF() carries all the information and is a bit shorter.
Quoted 1 line
+do { int delay = 1; \
We should at least prefix the delay with tst_ to make sure that it will not alias with anything that has been passed to the FUNC, e.g. if the FUNC is defined as foo_func(delay); the delay variable will be aliased and the function will do something very unexpected.
include/tst_common.h line 51
Quoted 4 lines
include/tst_common.h +4
-0+for (;;) { \ +typeof (FUNC) ret =FUNC ; \ +if (ret == (typeof (FUNC ))ERET ) \ +break ; \
Do we really need the (typeof(FUNC)) cast here?
include/tst_common.h line 54
Quoted 3 lines
include/tst_common.h +3
-0+if (delay <MAX_DELAY *1000000 ) { \ +usleep (delay); \ + delay *=2 ; \
Maybe we can be a bit more verbose and say something as:
tst_res(TINFO, #FUNC" returned %i, retrying in %ius", ret, delay);
include/tst_common.h line 57
Quoted 3 lines
include/tst_common.h +3
-0+ }else { \ +tst_brk_ (__FILE__, __LINE__, \ + TBROK | TERRNO, #FUNC " failed"); \
As far as I can tell we can just use the plain tst_brk() in macros since the __LINE__ will be a constant in the whole macro and will represent the line where the macro is called.
Also I'm not sure that adding the TERRNO here is a good idea, I suppose that there may be a retry functions that are not seting it on a failure. Maybe we can pass additional tst_res/tst_brk flags to the macro itself.
Quoted 4 lines
include/tst_common.h +4
-0+ } \ + } \ +}while (0 ) +
include/tst_common.h line 71
Quoted 10 lines
include/tst_common.h +10
-0+/* +* Exponential backoff usleep for wating a varible change +* @VAR: the variable will be changed in other place +* @EXP: an expected value which VAR should be equal to +*/ +#define TST_WAIT_ON_VAR (VAR ,EXP ) \ + TST_WAIT_WITH_EXPONENTIAL_BACKOFF(VAR, EXP, 1) + +#define TST_WAIT_WITH_EXPONENTIAL_BACKOFF (VAR ,EXP ,MAX_DELAY ) \ +do { int delay = 1; \
^ Trailing whitespace.
include/tst_common.h line 83
Quoted 12 lines
include/tst_common.h +12
-0+for (;;) { \ +if (VAR == (typeof (VAR ))EXP ) \ +break ; \ +if (delay <MAX_DELAY *1000000 ) { \ +usleep (delay); \ + delay *=2 ; \ + }else { \ +tst_brk_ (__FILE__, __LINE__, \ + TBROK | TERRNO, #VAR " is not expected"); \ + } \ + } \ +}while (0 )
I would refrain from adding this function unless we have a use-case already. Do you have a test in mind that could use this?
Quoted 4 lines
#endif /*
TST_COMMON_H__*/Signature · 1 line
1.9.3
Signature · 2 lines
Cyril Hrubis [email protected]
RFC PATCH 1/1 include: add two exponential backoff macros
Quoted 6 lines · Cyril Hrubis
+#define
TST_RETRY_FUNC_WITH_EXPONENTIAL_BACKOFF(FUNC, ERET, MAX_DELAY)\That is quite a long name, I guess that we can shorten it a bit without loosing too much clarity. Something as
TST_RETRY_FN_EXP_BACKOFF()carries all the information and is a bit shorter.
Yes, and we can also take use this directly for looping more seconds.
Quoted 7 lines
Quoted 1 line
+do { int delay = 1; \
We should at least prefix the delay with
tst_to make sure that it will not alias with anything that has been passed to the FUNC, e.g. if the FUNC is defined asfoo_func(delay); the delay variable will be aliased and the function will do something very unexpected.
You are right, I missed this problem.
Quoted 7 lines
include/tst_common.h line 51
Quoted 4 lines
include/tst_common.h +4
-0+for (;;) { \ +typeof (FUNC) ret =FUNC ; \ +if (ret == (typeof (FUNC ))ERET ) \ +break ; \Do we really need the (typeof(FUNC)) cast here?
I set the type cast to ensure more safe in comparing, but it seems no needed.
Will remove this.
Quoted 8 lines
include/tst_common.h line 54
Quoted 3 lines
include/tst_common.h +3
-0+if (delay <MAX_DELAY *1000000 ) { \ +usleep (delay); \ + delay *=2 ; \Maybe we can be a bit more verbose and say something as:
tst_res(TINFO, #FUNC" returned %i, retrying in %ius", ret, delay);
Agree.
Quoted 4 lines
include/tst_common.h line 57
Quoted 3 lines
include/tst_common.h +3
-0+ }else { \ +tst_brk_ (__FILE__, __LINE__, \ + TBROK | TERRNO, #FUNC " failed"); \
Quoted 9 lines
As far as I can tell we can just use the plain
tst_brk()in macros since the__LINE__will be a constant in the whole macro and will represent the line where the macro is called.Also I'm not sure that adding the TERRNO here is a good idea, I suppose that there may be a retry functions that are not seting it on a failure. Maybe we can pass additional tst_res/tst_brk flags to the macro itself.
Hmm, yes! But I don't want to add more flags to the macro since that will make things more complicated for user. So, let's just abandon the TERRNO to see how thing going on.
Quoted 29 lines
Quoted 4 lines
include/tst_common.h +4
-0+ } \ + } \ +}while (0 ) +include/tst_common.h line 71
Quoted 10 lines
include/tst_common.h +10
-0+/* +* Exponential backoff usleep for wating a varible change +* @VAR: the variable will be changed in other place +* @EXP: an expected value which VAR should be equal to +*/ +#define TST_WAIT_ON_VAR (VAR ,EXP ) \ + TST_WAIT_WITH_EXPONENTIAL_BACKOFF(VAR, EXP, 1) + +#define TST_WAIT_WITH_EXPONENTIAL_BACKOFF (VAR ,EXP ,MAX_DELAY ) \ +do { int delay = 1; \^ Trailing whitespace.include/tst_common.h line 83
Quoted 12 lines
include/tst_common.h +12
-0+for (;;) { \ +if (VAR == (typeof (VAR ))EXP ) \ +break ; \ +if (delay <MAX_DELAY *1000000 ) { \ +usleep (delay); \ + delay *=2 ; \ + }else { \ +tst_brk_ (__FILE__, __LINE__, \ + TBROK | TERRNO, #VAR " is not expected"); \ + } \ + } \ +}while (0 )
Quoted 3 lines
I would refrain from adding this function unless we have a use-case already. Do you have a test in mind that could use this?
No, I don't have any test so far, this is just came to my mind for one situation. We could re-add it if necessary next time.
Quoted 9 lines
Quoted 4 lines
#endif /*
TST_COMMON_H__*/
--
1.9.3--
Cyril Hrubis
[email protected]
Signature · 5 lines
Li Wang [email protected] -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.linux.it/pipermail/ltp/attachments/20180413/972e83a6/attachment-0001.html>