Re: [PATCH v3 1/6] scsi: ufs: Remove stringize operator '#' restriction

2020-12-14 Thread Steven Rostedt
On Mon, 14 Dec 2020 23:11:40 +
David Laight  wrote:

> > ---
> >  include/trace/events/ufs.h | 40 +++---
> >  1 file changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> > index 0bd54a184391..fa755394bc0f 100644
> > --- a/include/trace/events/ufs.h
> > +++ b/include/trace/events/ufs.h
> > @@ -20,28 +20,28 @@  
> ..
> > +#define UFS_LINK_STATES\
> > +   EM(UIC_LINK_OFF_STATE,  "UIC_LINK_OFF_STATE")   \
> > +   EM(UIC_LINK_ACTIVE_STATE,   "UIC_LINK_ACTIVE_STATE")\
> > +   EMe(UIC_LINK_HIBERN8_STATE, "UIC_LINK_HIBERN8_STATE")  
> 
> If you make EM a parameter to UFS_LINK_STATES then the caller
> can pass in the name of a #define that does the required expansion.
> The caller can also add in any required terminator after the last entry.
> For an enum (which doesn't want a , at the end) just add a dummy entry.
> You often want a constant for the number of items anyway.

This is currently the way its done in multiple other places. When creating
the "EMe" trick, I've thought about it, but then realized it would make the
other locations look strange without the expected comma, and decided that
EMe() would be the best solution, as it's only strange in where it's added,
and not where its used.

 $ git grep -l EMe include/trace/ |wc -l
 11

It's already used in 11 other files, let's not muck with it now.

-- Steve



RE: [PATCH v3 1/6] scsi: ufs: Remove stringize operator '#' restriction

2020-12-14 Thread David Laight
From: Bean Huo 
> Sent: 14 December 2020 20:20
> 
> From: Bean Huo 
> 
> Current EM macro definition, we use stringize operator '#', which turns
> the argument it precedes into a quoted string. Thus requires the symbol
> of __print_symbolic() should be the string corresponding to the name of
> the enum.
> 
> However, we have other cases, the symbol and enum name are not the same,
> we can redefine EM/EMe, but there will introduce some redundant codes.
> This patch is to remove this restriction, let others reuse the current
> EM/EMe definition.
> 
> Acked-by: Steven Rostedt (VMware) 
> Signed-off-by: Bean Huo 
> ---
>  include/trace/events/ufs.h | 40 +++---
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
> index 0bd54a184391..fa755394bc0f 100644
> --- a/include/trace/events/ufs.h
> +++ b/include/trace/events/ufs.h
> @@ -20,28 +20,28 @@
..
> +#define UFS_LINK_STATES  \
> + EM(UIC_LINK_OFF_STATE,  "UIC_LINK_OFF_STATE")   \
> + EM(UIC_LINK_ACTIVE_STATE,   "UIC_LINK_ACTIVE_STATE")\
> + EMe(UIC_LINK_HIBERN8_STATE, "UIC_LINK_HIBERN8_STATE")

If you make EM a parameter to UFS_LINK_STATES then the caller
can pass in the name of a #define that does the required expansion.
The caller can also add in any required terminator after the last entry.
For an enum (which doesn't want a , at the end) just add a dummy entry.
You often want a constant for the number of items anyway.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)



Re: [PATCH v3 1/6] scsi: ufs: Remove stringize operator '#' restriction

2020-12-14 Thread Bean Huo
On Mon, 2020-12-14 at 13:23 -0800, Joe Perches wrote:
> > From: Bean Huo 
> > 
> > Current EM macro definition, we use stringize operator '#', which
> > turns
> > the argument it precedes into a quoted string. Thus requires the
> > symbol
> > of __print_symbolic() should be the string corresponding to the
> > name of
> > the enum.
> > 
> > However, we have other cases, the symbol and enum name are not the
> > same,
> > we can redefine EM/EMe, but there will introduce some redundant
> > codes.
> > This patch is to remove this restriction, let others reuse the
> > current
> > EM/EMe definition.
> 
> While this version doesn't have the copy/paste typo,
> I fail to see value in defining EMe as a trailing comma
> in an array declaration isn't meaningful and doesn't emit
> any error or warning.
> 
> Maybe all the uses of EMe can be converted to EM and the
> macro definitions removed.

Hi Joe
I removed EMe, but there is this error:

./include/trace/trace_events.h:300:18: error: initializer element is
not constant
{ symbol_array, { -1, NULL }};   \

./include/trace/trace_events.h:300:18: error: expected expression
before ‘,’ token
{ symbol_array, { -1, NULL }};   \


did you choose kernel trace and event trace before compiling?


Thanks,
Bean



Re: [PATCH v3 1/6] scsi: ufs: Remove stringize operator '#' restriction

2020-12-14 Thread Joe Perches
On Mon, 2020-12-14 at 21:20 +0100, Bean Huo wrote:
> From: Bean Huo 
> 
> Current EM macro definition, we use stringize operator '#', which turns
> the argument it precedes into a quoted string. Thus requires the symbol
> of __print_symbolic() should be the string corresponding to the name of
> the enum.
> 
> However, we have other cases, the symbol and enum name are not the same,
> we can redefine EM/EMe, but there will introduce some redundant codes.
> This patch is to remove this restriction, let others reuse the current
> EM/EMe definition.

While this version doesn't have the copy/paste typo,
I fail to see value in defining EMe as a trailing comma
in an array declaration isn't meaningful and doesn't emit
any error or warning.

Maybe all the uses of EMe can be converted to EM and the
macro definitions removed.
---
 include/ras/ras_event.h|  64 -
 include/trace/events/9p.h  | 142 ++---
 include/trace/events/btrfs.h   |  66 -
 include/trace/events/huge_memory.h |  58 +++
 include/trace/events/migrate.h |  26 +++
 include/trace/events/mmflags.h |  34 -
 include/trace/events/sock.h|  56 +++
 include/trace/events/sunrpc.h  |  44 ++--
 include/trace/events/tlb.h |  16 ++---
 include/trace/events/ufs.h |  12 ++--
 include/trace/events/v4l2.h|  54 +++---
 include/trace/events/writeback.h   |  24 +++
 12 files changed, 274 insertions(+), 322 deletions(-)

diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index 0bdbc0d17d2f..32521a1066c7 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -348,56 +348,52 @@ TRACE_EVENT(aer_event,
 
 #ifdef CONFIG_MEMORY_FAILURE
 #define MF_ACTION_RESULT   \
-   EM ( MF_IGNORED, "Ignored" )\
-   EM ( MF_FAILED,  "Failed" ) \
-   EM ( MF_DELAYED, "Delayed" )\
-   EMe ( MF_RECOVERED, "Recovered" )
-
-#define MF_PAGE_TYPE   \
-   EM ( MF_MSG_KERNEL, "reserved kernel page" )\
-   EM ( MF_MSG_KERNEL_HIGH_ORDER, "high-order kernel page" )   \
-   EM ( MF_MSG_SLAB, "kernel slab page" )  \
-   EM ( MF_MSG_DIFFERENT_COMPOUND, "different compound page after locking" 
) \
-   EM ( MF_MSG_POISONED_HUGE, "huge page already hardware poisoned" )  
\
-   EM ( MF_MSG_HUGE, "huge page" ) \
-   EM ( MF_MSG_FREE_HUGE, "free huge page" )   \
-   EM ( MF_MSG_NON_PMD_HUGE, "non-pmd-sized huge page" )   \
-   EM ( MF_MSG_UNMAP_FAILED, "unmapping failed page" ) \
-   EM ( MF_MSG_DIRTY_SWAPCACHE, "dirty swapcache page" )   \
-   EM ( MF_MSG_CLEAN_SWAPCACHE, "clean swapcache page" )   \
-   EM ( MF_MSG_DIRTY_MLOCKED_LRU, "dirty mlocked LRU page" )   \
-   EM ( MF_MSG_CLEAN_MLOCKED_LRU, "clean mlocked LRU page" )   \
-   EM ( MF_MSG_DIRTY_UNEVICTABLE_LRU, "dirty unevictable LRU page" )   
\
-   EM ( MF_MSG_CLEAN_UNEVICTABLE_LRU, "clean unevictable LRU page" )   
\
-   EM ( MF_MSG_DIRTY_LRU, "dirty LRU page" )   \
-   EM ( MF_MSG_CLEAN_LRU, "clean LRU page" )   \
-   EM ( MF_MSG_TRUNCATED_LRU, "already truncated LRU page" )   \
-   EM ( MF_MSG_BUDDY, "free buddy page" )  \
-   EM ( MF_MSG_BUDDY_2ND, "free buddy page (2nd try)" )\
-   EM ( MF_MSG_DAX, "dax page" )   \
-   EM ( MF_MSG_UNSPLIT_THP, "unsplit thp" )\
-   EMe ( MF_MSG_UNKNOWN, "unknown page" )
+   EM(MF_IGNORED, "Ignored")   \
+   EM(MF_FAILED,  "Failed")\
+   EM(MF_DELAYED, "Delayed")   \
+   EM(MF_RECOVERED, "Recovered")
+
+#define MF_PAGE_TYPE   \
+   EM(MF_MSG_KERNEL, "reserved kernel page")   \
+   EM(MF_MSG_KERNEL_HIGH_ORDER, "high-order kernel page")  \
+   EM(MF_MSG_SLAB, "kernel slab page") \
+   EM(MF_MSG_DIFFERENT_COMPOUND, "different compound page after locking") \
+   EM(MF_MSG_POISONED_HUGE, "huge page already hardware poisoned") \
+   EM(MF_MSG_HUGE, "huge page")\
+   EM(MF_MSG_FREE_HUGE, "free huge page")  \
+   EM(MF_MSG_NON_PMD_HUGE, "non-pmd-sized huge page")  \
+   EM(MF_MSG_UNMAP_FAILED, "unmapping failed page")\
+   EM(MF_MSG_DIRTY_SWAPCACHE, "dirty swapcache page")  \
+   EM(MF_MSG_CLEAN_SWAPCACHE, "clean swapcache page")  \
+   EM(MF_MSG_DIRTY_MLOCKED_LRU, "dirty mlocked LRU page")  \
+   EM(MF_MSG_CLEAN_MLOCKED_LRU, "clean mlocked LRU page")  \
+   

[PATCH v3 1/6] scsi: ufs: Remove stringize operator '#' restriction

2020-12-14 Thread Bean Huo
From: Bean Huo 

Current EM macro definition, we use stringize operator '#', which turns
the argument it precedes into a quoted string. Thus requires the symbol
of __print_symbolic() should be the string corresponding to the name of
the enum.

However, we have other cases, the symbol and enum name are not the same,
we can redefine EM/EMe, but there will introduce some redundant codes.
This patch is to remove this restriction, let others reuse the current
EM/EMe definition.

Acked-by: Steven Rostedt (VMware) 
Signed-off-by: Bean Huo 
---
 include/trace/events/ufs.h | 40 +++---
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/trace/events/ufs.h b/include/trace/events/ufs.h
index 0bd54a184391..fa755394bc0f 100644
--- a/include/trace/events/ufs.h
+++ b/include/trace/events/ufs.h
@@ -20,28 +20,28 @@
{ SYNCHRONIZE_CACHE,"SYNC" },   \
{ UNMAP,"UNMAP" })
 
-#define UFS_LINK_STATES\
-   EM(UIC_LINK_OFF_STATE)  \
-   EM(UIC_LINK_ACTIVE_STATE)   \
-   EMe(UIC_LINK_HIBERN8_STATE)
-
-#define UFS_PWR_MODES  \
-   EM(UFS_ACTIVE_PWR_MODE) \
-   EM(UFS_SLEEP_PWR_MODE)  \
-   EM(UFS_POWERDOWN_PWR_MODE)  \
-   EMe(UFS_DEEPSLEEP_PWR_MODE)
-
-#define UFSCHD_CLK_GATING_STATES   \
-   EM(CLKS_OFF)\
-   EM(CLKS_ON) \
-   EM(REQ_CLKS_OFF)\
-   EMe(REQ_CLKS_ON)
+#define UFS_LINK_STATES\
+   EM(UIC_LINK_OFF_STATE,  "UIC_LINK_OFF_STATE")   \
+   EM(UIC_LINK_ACTIVE_STATE,   "UIC_LINK_ACTIVE_STATE")\
+   EMe(UIC_LINK_HIBERN8_STATE, "UIC_LINK_HIBERN8_STATE")
+
+#define UFS_PWR_MODES  \
+   EM(UFS_ACTIVE_PWR_MODE, "UFS_ACTIVE_PWR_MODE")  \
+   EM(UFS_SLEEP_PWR_MODE,  "UFS_SLEEP_PWR_MODE")   \
+   EM(UFS_POWERDOWN_PWR_MODE,  "UFS_POWERDOWN_PWR_MODE")   \
+   EMe(UFS_DEEPSLEEP_PWR_MODE, "UFS_DEEPSLEEP_PWR_MODE")
+
+#define UFSCHD_CLK_GATING_STATES   \
+   EM(CLKS_OFF,"CLKS_OFF") \
+   EM(CLKS_ON, "CLKS_ON")  \
+   EM(REQ_CLKS_OFF,"REQ_CLKS_OFF") \
+   EMe(REQ_CLKS_ON,"REQ_CLKS_ON")
 
 /* Enums require being exported to userspace, for user tool parsing */
 #undef EM
 #undef EMe
-#define EM(a)  TRACE_DEFINE_ENUM(a);
-#define EMe(a) TRACE_DEFINE_ENUM(a);
+#define EM(a, b)   TRACE_DEFINE_ENUM(a);
+#define EMe(a, b)  TRACE_DEFINE_ENUM(a);
 
 UFS_LINK_STATES;
 UFS_PWR_MODES;
@@ -53,8 +53,8 @@ UFSCHD_CLK_GATING_STATES;
  */
 #undef EM
 #undef EMe
-#define EM(a)  { a, #a },
-#define EMe(a) { a, #a }
+#define EM(a, b)   {a, b},
+#define EMe(a, b)  {a, b}
 
 TRACE_EVENT(ufshcd_clk_gating,
 
-- 
2.17.1