| Message ID | 20210922071811.1913-6-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Cleanup and maintenance 2 | expand |
On Wed, 22 Sep 2021 07:18:07 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > Every time something goes wrong in a way we don't expect, we need to > add debug prints to some UVC to get the unexpected return code. > > Let's just put the printing behind a macro so we can enable it if > needed via a simple switch. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> but see a nit below > --- > lib/s390x/asm/uv.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h > index 2f099553..0e958ad7 100644 > --- a/lib/s390x/asm/uv.h > +++ b/lib/s390x/asm/uv.h > @@ -12,6 +12,9 @@ > #ifndef _ASMS390X_UV_H_ > #define _ASMS390X_UV_H_ > > +/* Enables printing of command code and return codes for failed UVCs > */ +#define UVC_ERR_DEBUG 0 > + > #define UVC_RC_EXECUTED 0x0001 > #define UVC_RC_INV_CMD 0x0002 > #define UVC_RC_INV_STATE 0x0003 > @@ -194,6 +197,15 @@ static inline int uv_call_once(unsigned long r1, > unsigned long r2) : [cc] "=d" (cc) > : [r1] "a" (r1), [r2] "a" (r2) > : "memory", "cc"); > + > +#if UVC_ERR_DEBUG > + if (cc) it probably looks cleaner like this: if (UVC_ERR_DEBUG && cc) and without the #if; the compiler should be smart enough to remove the dead code. In practice it doesn't really matter in the end, so feel free to ignore this comment :) > + printf("UV call error: call %x rc %x rrc %x\n", > + ((struct uv_cb_header *)r2)->cmd, > + ((struct uv_cb_header *)r2)->rc, > + ((struct uv_cb_header *)r2)->rrc); > +#endif > + > return cc; > } >
On 9/22/21 11:23 AM, Claudio Imbrenda wrote: > On Wed, 22 Sep 2021 07:18:07 +0000 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> Every time something goes wrong in a way we don't expect, we need to >> add debug prints to some UVC to get the unexpected return code. >> >> Let's just put the printing behind a macro so we can enable it if >> needed via a simple switch. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > but see a nit below > >> --- >> lib/s390x/asm/uv.h | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h >> index 2f099553..0e958ad7 100644 >> --- a/lib/s390x/asm/uv.h >> +++ b/lib/s390x/asm/uv.h >> @@ -12,6 +12,9 @@ >> #ifndef _ASMS390X_UV_H_ >> #define _ASMS390X_UV_H_ >> >> +/* Enables printing of command code and return codes for failed UVCs >> */ +#define UVC_ERR_DEBUG 0 >> + >> #define UVC_RC_EXECUTED 0x0001 >> #define UVC_RC_INV_CMD 0x0002 >> #define UVC_RC_INV_STATE 0x0003 >> @@ -194,6 +197,15 @@ static inline int uv_call_once(unsigned long r1, >> unsigned long r2) : [cc] "=d" (cc) >> : [r1] "a" (r1), [r2] "a" (r2) >> : "memory", "cc"); >> + >> +#if UVC_ERR_DEBUG >> + if (cc) > > it probably looks cleaner like this: > > if (UVC_ERR_DEBUG && cc) > > and without the #if; the compiler should be smart enough to remove the > dead code. In practice it doesn't really matter in the end, so feel free > to ignore this comment :) Didn't consider that, I'll fix it up. Thank you > >> + printf("UV call error: call %x rc %x rrc %x\n", >> + ((struct uv_cb_header *)r2)->cmd, >> + ((struct uv_cb_header *)r2)->rc, >> + ((struct uv_cb_header *)r2)->rrc); >> +#endif >> + >> return cc; >> } >> >
On 22/09/2021 09.18, Janosch Frank wrote: > Every time something goes wrong in a way we don't expect, we need to > add debug prints to some UVC to get the unexpected return code. > > Let's just put the printing behind a macro so we can enable it if > needed via a simple switch. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/asm/uv.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h > index 2f099553..0e958ad7 100644 > --- a/lib/s390x/asm/uv.h > +++ b/lib/s390x/asm/uv.h > @@ -12,6 +12,9 @@ > #ifndef _ASMS390X_UV_H_ > #define _ASMS390X_UV_H_ > > +/* Enables printing of command code and return codes for failed UVCs */ > +#define UVC_ERR_DEBUG 0 Do we maybe want a "#ifndef UVC_ERR_DEBUG" in front of this, so that we could also set the macro to 1 from individual *.c files (or from the Makefile)? Thomas
On 9/27/21 19:41, Thomas Huth wrote: > On 22/09/2021 09.18, Janosch Frank wrote: >> Every time something goes wrong in a way we don't expect, we need to >> add debug prints to some UVC to get the unexpected return code. >> >> Let's just put the printing behind a macro so we can enable it if >> needed via a simple switch. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> lib/s390x/asm/uv.h | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h >> index 2f099553..0e958ad7 100644 >> --- a/lib/s390x/asm/uv.h >> +++ b/lib/s390x/asm/uv.h >> @@ -12,6 +12,9 @@ >> #ifndef _ASMS390X_UV_H_ >> #define _ASMS390X_UV_H_ >> >> +/* Enables printing of command code and return codes for failed UVCs */ >> +#define UVC_ERR_DEBUG 0 > > Do we maybe want a "#ifndef UVC_ERR_DEBUG" in front of this, so that we > could also set the macro to 1 from individual *.c files (or from the Makefile)? > > Thomas > When testing it's the least amount of work to set this to 1 so I implemented it this way. If you think it's useful then I'll add the ifndef.
diff --git a/lib/s390x/asm/uv.h b/lib/s390x/asm/uv.h index 2f099553..0e958ad7 100644 --- a/lib/s390x/asm/uv.h +++ b/lib/s390x/asm/uv.h @@ -12,6 +12,9 @@ #ifndef _ASMS390X_UV_H_ #define _ASMS390X_UV_H_ +/* Enables printing of command code and return codes for failed UVCs */ +#define UVC_ERR_DEBUG 0 + #define UVC_RC_EXECUTED 0x0001 #define UVC_RC_INV_CMD 0x0002 #define UVC_RC_INV_STATE 0x0003 @@ -194,6 +197,15 @@ static inline int uv_call_once(unsigned long r1, unsigned long r2) : [cc] "=d" (cc) : [r1] "a" (r1), [r2] "a" (r2) : "memory", "cc"); + +#if UVC_ERR_DEBUG + if (cc) + printf("UV call error: call %x rc %x rrc %x\n", + ((struct uv_cb_header *)r2)->cmd, + ((struct uv_cb_header *)r2)->rc, + ((struct uv_cb_header *)r2)->rrc); +#endif + return cc; }
Every time something goes wrong in a way we don't expect, we need to add debug prints to some UVC to get the unexpected return code. Let's just put the printing behind a macro so we can enable it if needed via a simple switch. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/asm/uv.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)