* [PATCH 1/1] RDMA/rxe: Add function name to the logs
@ 2023-04-18 12:26 Zhu Yanjun
2023-04-21 15:27 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Zhu Yanjun @ 2023-04-18 12:26 UTC (permalink / raw)
To: zyjzyj2000, jgg, leon, linux-rdma; +Cc: Zhu Yanjun
From: Zhu Yanjun <yanjun.zhu@linux.dev>
Add the function names to the pr_ logs. As such, if some bugs occur,
with function names, it is easy to locate the bugs.
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
drivers/infiniband/sw/rxe/rxe.h | 2 +-
drivers/infiniband/sw/rxe/rxe_queue.h | 16 ++++------------
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index d33dd6cf83d3..f897d0ac1216 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -10,7 +10,7 @@
#ifdef pr_fmt
#undef pr_fmt
#endif
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
#include <linux/skbuff.h>
diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
index c711cb98b949..11bba08ca5f9 100644
--- a/drivers/infiniband/sw/rxe/rxe_queue.h
+++ b/drivers/infiniband/sw/rxe/rxe_queue.h
@@ -184,9 +184,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
switch (type) {
case QUEUE_TYPE_FROM_CLIENT:
/* used by rxe, client owns the index */
- if (WARN_ON(1))
- pr_warn("%s: attempt to advance client index\n",
- __func__);
+ WARN_ON(1);
break;
case QUEUE_TYPE_TO_CLIENT:
/* used by rxe which owns the index */
@@ -205,9 +203,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
break;
case QUEUE_TYPE_TO_ULP:
/* used by ulp, rxe owns the index */
- if (WARN_ON(1))
- pr_warn("%s: attempt to advance driver index\n",
- __func__);
+ WARN_ON(1);
break;
}
}
@@ -227,15 +223,11 @@ static inline void queue_advance_consumer(struct rxe_queue *q,
break;
case QUEUE_TYPE_TO_CLIENT:
/* used by rxe, client owns the index */
- if (WARN_ON(1))
- pr_warn("%s: attempt to advance client index\n",
- __func__);
+ WARN_ON(1);
break;
case QUEUE_TYPE_FROM_ULP:
/* used by ulp, rxe owns the index */
- if (WARN_ON(1))
- pr_warn("%s: attempt to advance driver index\n",
- __func__);
+ WARN_ON(1);
break;
case QUEUE_TYPE_TO_ULP:
/* used by ulp which owns the index */
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-18 12:26 [PATCH 1/1] RDMA/rxe: Add function name to the logs Zhu Yanjun
@ 2023-04-21 15:27 ` Jason Gunthorpe
2023-04-21 15:51 ` Zhu Yanjun
0 siblings, 1 reply; 8+ messages in thread
From: Jason Gunthorpe @ 2023-04-21 15:27 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: zyjzyj2000, leon, linux-rdma, Zhu Yanjun
On Tue, Apr 18, 2023 at 08:26:11PM +0800, Zhu Yanjun wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> Add the function names to the pr_ logs. As such, if some bugs occur,
> with function names, it is easy to locate the bugs.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
> drivers/infiniband/sw/rxe/rxe.h | 2 +-
> drivers/infiniband/sw/rxe/rxe_queue.h | 16 ++++------------
> 2 files changed, 5 insertions(+), 13 deletions(-)
After you delete the warn_on's there is very little left:
rivers/infiniband/sw/rxe/rxe.c: pr_err("rxe creation allowed on top of a real device only\n");
drivers/infiniband/sw/rxe/rxe.c: pr_info("loaded\n");
drivers/infiniband/sw/rxe/rxe.c: pr_info("unloaded\n");
drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv4 UDP tunnel\n");
drivers/infiniband/sw/rxe/rxe_net.c: pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n");
drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv6 UDP tunnel\n");
drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to register netdev notifier\n");
It is not exactly mysterious where these come from?
Why do we need the function name?
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-21 15:27 ` Jason Gunthorpe
@ 2023-04-21 15:51 ` Zhu Yanjun
2023-04-28 13:24 ` Jason Gunthorpe
0 siblings, 1 reply; 8+ messages in thread
From: Zhu Yanjun @ 2023-04-21 15:51 UTC (permalink / raw)
To: Jason Gunthorpe, Zhu Yanjun; +Cc: zyjzyj2000, leon, linux-rdma
在 2023/4/21 23:27, Jason Gunthorpe 写道:
> On Tue, Apr 18, 2023 at 08:26:11PM +0800, Zhu Yanjun wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> Add the function names to the pr_ logs. As such, if some bugs occur,
>> with function names, it is easy to locate the bugs.
>>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>> drivers/infiniband/sw/rxe/rxe.h | 2 +-
>> drivers/infiniband/sw/rxe/rxe_queue.h | 16 ++++------------
>> 2 files changed, 5 insertions(+), 13 deletions(-)
> After you delete the warn_on's there is very little left:
>
> rivers/infiniband/sw/rxe/rxe.c: pr_err("rxe creation allowed on top of a real device only\n");
> drivers/infiniband/sw/rxe/rxe.c: pr_info("loaded\n");
> drivers/infiniband/sw/rxe/rxe.c: pr_info("unloaded\n");
> drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv4 UDP tunnel\n");
> drivers/infiniband/sw/rxe/rxe_net.c: pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n");
> drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv6 UDP tunnel\n");
> drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to register netdev notifier\n");
>
> It is not exactly mysterious where these come from?
If I got you correctly, you mean that we can easily locate the above
pr_xxx logs. As such, it is not necessary to add function names.
From this perspective, I agree with you.
Why I make this commit is that we can directly use this pr_info("xxxxx")
when some bug occurs. But in the logs, the function names appear.
With this commit, we can decrease some work loads with some bug occurs.
Now I am debugging a problem. With a similar commit with this, I
directly use pr_warn("xxx"). But in the logs, function names, line
number, file names, caller and so on will appear.
This greatly decrease my workloads.
I think this is a smart method. So I introduce this smart method to
softroce. When some bugs occur in softroce, this method can help us.
Zhu Yanjun
>
> Why do we need the function name?
>
> Jason
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-21 15:51 ` Zhu Yanjun
@ 2023-04-28 13:24 ` Jason Gunthorpe
0 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2023-04-28 13:24 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: Zhu Yanjun, zyjzyj2000, leon, linux-rdma
On Fri, Apr 21, 2023 at 11:51:34PM +0800, Zhu Yanjun wrote:
>
> 在 2023/4/21 23:27, Jason Gunthorpe 写道:
> > On Tue, Apr 18, 2023 at 08:26:11PM +0800, Zhu Yanjun wrote:
> > > From: Zhu Yanjun <yanjun.zhu@linux.dev>
> > >
> > > Add the function names to the pr_ logs. As such, if some bugs occur,
> > > with function names, it is easy to locate the bugs.
> > >
> > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> > > ---
> > > drivers/infiniband/sw/rxe/rxe.h | 2 +-
> > > drivers/infiniband/sw/rxe/rxe_queue.h | 16 ++++------------
> > > 2 files changed, 5 insertions(+), 13 deletions(-)
> > After you delete the warn_on's there is very little left:
> >
> > rivers/infiniband/sw/rxe/rxe.c: pr_err("rxe creation allowed on top of a real device only\n");
> > drivers/infiniband/sw/rxe/rxe.c: pr_info("loaded\n");
> > drivers/infiniband/sw/rxe/rxe.c: pr_info("unloaded\n");
> > drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv4 UDP tunnel\n");
> > drivers/infiniband/sw/rxe/rxe_net.c: pr_warn("IPv6 is not supported, can not create a UDPv6 socket\n");
> > drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to create IPv6 UDP tunnel\n");
> > drivers/infiniband/sw/rxe/rxe_net.c: pr_err("Failed to register netdev notifier\n");
> >
> > It is not exactly mysterious where these come from?
>
> If I got you correctly, you mean that we can easily locate the above pr_xxx
> logs. As such, it is not necessary to add function names.
>
> From this perspective, I agree with you.
>
> Why I make this commit is that we can directly use this pr_info("xxxxx")
> when some bug occurs. But in the logs, the function names appear.
>
> With this commit, we can decrease some work loads with some bug occurs.
That doesn't seem like a good reason to merge something so abnormal
for pr_* functions
Jason
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/1] RDMA/rxe: Add function name to the logs
@ 2023-04-10 10:21 Zhu Yanjun
2023-04-16 14:43 ` Zhu Yanjun
2023-04-18 8:08 ` Leon Romanovsky
0 siblings, 2 replies; 8+ messages in thread
From: Zhu Yanjun @ 2023-04-10 10:21 UTC (permalink / raw)
To: zyjzyj2000, jgg, leon, linux-rdma; +Cc: Zhu Yanjun
From: Zhu Yanjun <yanjun.zhu@linux.dev>
Add the function names to the pr_ logs. As such, if some bugs occur,
with function names, it is easy to locate the bugs.
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
drivers/infiniband/sw/rxe/rxe.h | 2 +-
drivers/infiniband/sw/rxe/rxe_queue.h | 12 ++++--------
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index 2415f3704f57..43742d2f32de 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -10,7 +10,7 @@
#ifdef pr_fmt
#undef pr_fmt
#endif
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
#include <linux/skbuff.h>
diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
index c711cb98b949..5d6e17b00e60 100644
--- a/drivers/infiniband/sw/rxe/rxe_queue.h
+++ b/drivers/infiniband/sw/rxe/rxe_queue.h
@@ -185,8 +185,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
case QUEUE_TYPE_FROM_CLIENT:
/* used by rxe, client owns the index */
if (WARN_ON(1))
- pr_warn("%s: attempt to advance client index\n",
- __func__);
+ pr_warn("attempt to advance client index\n");
break;
case QUEUE_TYPE_TO_CLIENT:
/* used by rxe which owns the index */
@@ -206,8 +205,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
case QUEUE_TYPE_TO_ULP:
/* used by ulp, rxe owns the index */
if (WARN_ON(1))
- pr_warn("%s: attempt to advance driver index\n",
- __func__);
+ pr_warn("attempt to advance driver index\n");
break;
}
}
@@ -228,14 +226,12 @@ static inline void queue_advance_consumer(struct rxe_queue *q,
case QUEUE_TYPE_TO_CLIENT:
/* used by rxe, client owns the index */
if (WARN_ON(1))
- pr_warn("%s: attempt to advance client index\n",
- __func__);
+ pr_warn("attempt to advance client index\n");
break;
case QUEUE_TYPE_FROM_ULP:
/* used by ulp, rxe owns the index */
if (WARN_ON(1))
- pr_warn("%s: attempt to advance driver index\n",
- __func__);
+ pr_warn("attempt to advance driver index\n");
break;
case QUEUE_TYPE_TO_ULP:
/* used by ulp which owns the index */
--
2.27.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-10 10:21 Zhu Yanjun
@ 2023-04-16 14:43 ` Zhu Yanjun
2023-04-18 8:08 ` Leon Romanovsky
1 sibling, 0 replies; 8+ messages in thread
From: Zhu Yanjun @ 2023-04-16 14:43 UTC (permalink / raw)
To: Zhu Yanjun, zyjzyj2000, jgg, leon, linux-rdma
在 2023/4/10 18:21, Zhu Yanjun 写道:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> Add the function names to the pr_ logs. As such, if some bugs occur,
> with function names, it is easy to locate the bugs.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
Gently ping
Zhu Yanjun
> ---
> drivers/infiniband/sw/rxe/rxe.h | 2 +-
> drivers/infiniband/sw/rxe/rxe_queue.h | 12 ++++--------
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index 2415f3704f57..43742d2f32de 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -10,7 +10,7 @@
> #ifdef pr_fmt
> #undef pr_fmt
> #endif
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
>
> #include <linux/skbuff.h>
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
> index c711cb98b949..5d6e17b00e60 100644
> --- a/drivers/infiniband/sw/rxe/rxe_queue.h
> +++ b/drivers/infiniband/sw/rxe/rxe_queue.h
> @@ -185,8 +185,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
> case QUEUE_TYPE_FROM_CLIENT:
> /* used by rxe, client owns the index */
> if (WARN_ON(1))
> - pr_warn("%s: attempt to advance client index\n",
> - __func__);
> + pr_warn("attempt to advance client index\n");
> break;
> case QUEUE_TYPE_TO_CLIENT:
> /* used by rxe which owns the index */
> @@ -206,8 +205,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
> case QUEUE_TYPE_TO_ULP:
> /* used by ulp, rxe owns the index */
> if (WARN_ON(1))
> - pr_warn("%s: attempt to advance driver index\n",
> - __func__);
> + pr_warn("attempt to advance driver index\n");
> break;
> }
> }
> @@ -228,14 +226,12 @@ static inline void queue_advance_consumer(struct rxe_queue *q,
> case QUEUE_TYPE_TO_CLIENT:
> /* used by rxe, client owns the index */
> if (WARN_ON(1))
> - pr_warn("%s: attempt to advance client index\n",
> - __func__);
> + pr_warn("attempt to advance client index\n");
> break;
> case QUEUE_TYPE_FROM_ULP:
> /* used by ulp, rxe owns the index */
> if (WARN_ON(1))
> - pr_warn("%s: attempt to advance driver index\n",
> - __func__);
> + pr_warn("attempt to advance driver index\n");
> break;
> case QUEUE_TYPE_TO_ULP:
> /* used by ulp which owns the index */
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-10 10:21 Zhu Yanjun
2023-04-16 14:43 ` Zhu Yanjun
@ 2023-04-18 8:08 ` Leon Romanovsky
2023-04-18 10:36 ` Zhu Yanjun
1 sibling, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2023-04-18 8:08 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: zyjzyj2000, jgg, linux-rdma, Zhu Yanjun
On Mon, Apr 10, 2023 at 06:21:05PM +0800, Zhu Yanjun wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>
> Add the function names to the pr_ logs. As such, if some bugs occur,
> with function names, it is easy to locate the bugs.
>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
> drivers/infiniband/sw/rxe/rxe.h | 2 +-
> drivers/infiniband/sw/rxe/rxe_queue.h | 12 ++++--------
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index 2415f3704f57..43742d2f32de 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -10,7 +10,7 @@
> #ifdef pr_fmt
> #undef pr_fmt
> #endif
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
>
> #include <linux/skbuff.h>
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
> index c711cb98b949..5d6e17b00e60 100644
> --- a/drivers/infiniband/sw/rxe/rxe_queue.h
> +++ b/drivers/infiniband/sw/rxe/rxe_queue.h
> @@ -185,8 +185,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
> case QUEUE_TYPE_FROM_CLIENT:
> /* used by rxe, client owns the index */
> if (WARN_ON(1))
> - pr_warn("%s: attempt to advance client index\n",
> - __func__);
> + pr_warn("attempt to advance client index\n");
Delete all if (WARN_ON(1)) pr_warn(...) in favour of plain WARN_ON().
It will give you all information which you need.
Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/1] RDMA/rxe: Add function name to the logs
2023-04-18 8:08 ` Leon Romanovsky
@ 2023-04-18 10:36 ` Zhu Yanjun
0 siblings, 0 replies; 8+ messages in thread
From: Zhu Yanjun @ 2023-04-18 10:36 UTC (permalink / raw)
To: Leon Romanovsky, Zhu Yanjun; +Cc: zyjzyj2000, jgg, linux-rdma
在 2023/4/18 16:08, Leon Romanovsky 写道:
> On Mon, Apr 10, 2023 at 06:21:05PM +0800, Zhu Yanjun wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> Add the function names to the pr_ logs. As such, if some bugs occur,
>> with function names, it is easy to locate the bugs.
>>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>> drivers/infiniband/sw/rxe/rxe.h | 2 +-
>> drivers/infiniband/sw/rxe/rxe_queue.h | 12 ++++--------
>> 2 files changed, 5 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
>> index 2415f3704f57..43742d2f32de 100644
>> --- a/drivers/infiniband/sw/rxe/rxe.h
>> +++ b/drivers/infiniband/sw/rxe/rxe.h
>> @@ -10,7 +10,7 @@
>> #ifdef pr_fmt
>> #undef pr_fmt
>> #endif
>> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__
>>
>> #include <linux/skbuff.h>
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_queue.h b/drivers/infiniband/sw/rxe/rxe_queue.h
>> index c711cb98b949..5d6e17b00e60 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_queue.h
>> +++ b/drivers/infiniband/sw/rxe/rxe_queue.h
>> @@ -185,8 +185,7 @@ static inline void queue_advance_producer(struct rxe_queue *q,
>> case QUEUE_TYPE_FROM_CLIENT:
>> /* used by rxe, client owns the index */
>> if (WARN_ON(1))
>> - pr_warn("%s: attempt to advance client index\n",
>> - __func__);
>> + pr_warn("attempt to advance client index\n");
> Delete all if (WARN_ON(1)) pr_warn(...) in favour of plain WARN_ON().
> It will give you all information which you need.
Got it. Thanks a lot. I will fix it ASAP.
Zhu Yanjun
>
> Thanks
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-04-28 13:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-18 12:26 [PATCH 1/1] RDMA/rxe: Add function name to the logs Zhu Yanjun
2023-04-21 15:27 ` Jason Gunthorpe
2023-04-21 15:51 ` Zhu Yanjun
2023-04-28 13:24 ` Jason Gunthorpe
-- strict thread matches above, loose matches on Subject: below --
2023-04-10 10:21 Zhu Yanjun
2023-04-16 14:43 ` Zhu Yanjun
2023-04-18 8:08 ` Leon Romanovsky
2023-04-18 10:36 ` Zhu Yanjun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox