| Message ID | 20240323083139.5484-1-yanjun.zhu@linux.dev (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] RDMA/rxe: Make pr_fmt work | expand |
On Sat, Mar 23, 2024 at 4:32 PM Yanjun.Zhu <yanjun.zhu@linux.dev> wrote: > > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > If the definition of pr_fmt is before the header file. The pr_fmt > will be overwritten by the header file. So move the definition of > pr_fmt to the below of the header file. Awesome. You fix my problem. > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > --- > drivers/infiniband/sw/rxe/rxe.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h > index d8fb2c7af30a..dc2d8dd2f681 100644 > --- a/drivers/infiniband/sw/rxe/rxe.h > +++ b/drivers/infiniband/sw/rxe/rxe.h > @@ -7,11 +7,6 @@ > #ifndef RXE_H > #define RXE_H > > -#ifdef pr_fmt > -#undef pr_fmt > -#endif > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > - > #include <linux/skbuff.h> > > #include <rdma/ib_verbs.h> > @@ -30,6 +25,11 @@ > #include "rxe_verbs.h" > #include "rxe_loc.h" > > +#ifdef pr_fmt > +#undef pr_fmt > +#endif > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > /* > * Version 1 and Version 2 are identical on 64 bit machines, but on 32 bit > * machines Version 2 has a different struct layout. > -- > 2.34.1 > >
On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > If the definition of pr_fmt is before the header file. The pr_fmt > will be overwritten by the header file. So move the definition of > pr_fmt to the below of the header file. what header file? Jason
在 2024/3/27 14:08, Jason Gunthorpe 写道: > On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: >> From: Zhu Yanjun <yanjun.zhu@linux.dev> >> >> If the definition of pr_fmt is before the header file. The pr_fmt >> will be overwritten by the header file. So move the definition of >> pr_fmt to the below of the header file. > what header file? include/linux/printk.h Because this driver will finally call printk function to output the logs, the header file include/linux/printk.h needs be included. In include/linux/printk.h, pr_fmt is defined. I made a simple checks. In about 130 header files, this pr_fmt is defined. Zhu Yanjun > > Jason
On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > If the definition of pr_fmt is before the header file. The pr_fmt > will be overwritten by the header file. So move the definition of > pr_fmt to the below of the header file. > > Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> > --- > drivers/infiniband/sw/rxe/rxe.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) Let's just convert RXE to ibdev*() prints instead. Thanks
在 2024/4/2 19:45, Leon Romanovsky 写道: > On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: >> From: Zhu Yanjun <yanjun.zhu@linux.dev> >> >> If the definition of pr_fmt is before the header file. The pr_fmt >> will be overwritten by the header file. So move the definition of >> pr_fmt to the below of the header file. >> >> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev> >> --- >> drivers/infiniband/sw/rxe/rxe.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) > > Let's just convert RXE to ibdev*() prints instead. OK. I will do. But it takes me some time because it seems a big task.^_^ Zhu Yanjun > > Thanks
On Wed, Mar 27, 2024 at 08:40:54PM +0100, Zhu Yanjun wrote: > > 在 2024/3/27 14:08, Jason Gunthorpe 写道: > > On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: > > > From: Zhu Yanjun <yanjun.zhu@linux.dev> > > > > > > If the definition of pr_fmt is before the header file. The pr_fmt > > > will be overwritten by the header file. So move the definition of > > > pr_fmt to the below of the header file. > > what header file? > > include/linux/printk.h > > Because this driver will finally call printk function to output the logs, > the header file include/linux/printk.h needs be included. > > In include/linux/printk.h, pr_fmt is defined. This doesn't make sense, printk.h has: #ifndef pr_fmt #define pr_fmt(fmt) fmt #endif Before or after printk.h should not have an impact. Jason
在 2024/4/4 20:01, Zhu Yanjun 写道: > > > 在 2024/4/4 16:59, Jason Gunthorpe 写道: >> On Wed, Mar 27, 2024 at 08:40:54PM +0100, Zhu Yanjun wrote: >>> 在 2024/3/27 14:08, Jason Gunthorpe 写道: >>>> On Sat, Mar 23, 2024 at 09:31:39AM +0100, Yanjun.Zhu wrote: >>>>> From: Zhu Yanjun<yanjun.zhu@linux.dev> >>>>> >>>>> If the definition of pr_fmt is before the header file. The pr_fmt >>>>> will be overwritten by the header file. So move the definition of >>>>> pr_fmt to the below of the header file. >>>> what header file? >>> include/linux/printk.h >>> >>> Because this driver will finally call printk function to output the logs, >>> the header file include/linux/printk.h needs be included. >>> >>> In include/linux/printk.h, pr_fmt is defined. >> This doesn't make sense, printk.h has: >> >> #ifndef pr_fmt >> #define pr_fmt(fmt) fmt >> #endif >> >> Before or after printk.h should not have an impact. Sorry. The previous mail is not sent successfully. I resend it. > #ifndef pr_fmt > > ... > > #endif > > The above will not undefine pr_fmt. > > #undef pr_fmt will undefine pr_fmt. > > This link explains the above in details. > https://www.techonthenet.com/c_language/directives/ifndef.php > > Thanks a lot for your comments. > > Zhu Yanjun > >> Jason
On Thu, Apr 04, 2024 at 08:03:35PM +0200, Zhu Yanjun wrote: > > > > Because this driver will finally call printk function to output the logs, > > > > the header file include/linux/printk.h needs be included. > > > > > > > > In include/linux/printk.h, pr_fmt is defined. > > > This doesn't make sense, printk.h has: > > > > > > #ifndef pr_fmt > > > #define pr_fmt(fmt) fmt > > > #endif > > > > > > Before or after printk.h should not have an impact. > > > Sorry. The previous mail is not sent successfully. I resend it. > > > #ifndef pr_fmt > > > > ... > > > > #endif > > > > The above will not undefine pr_fmt. > > > > #undef pr_fmt will undefine pr_fmt. > > > > This link explains the above in details. > > > https://www.techonthenet.com/c_language/directives/ifndef.php Why would you want to undefine it? The point is to #define it to the rxe specific value. If it is already set to the rxe specific value before including printk.h then it will work fine? Jason
在 2024/4/5 1:59, Jason Gunthorpe 写道: > On Thu, Apr 04, 2024 at 08:03:35PM +0200, Zhu Yanjun wrote: >>>>> Because this driver will finally call printk function to output the logs, >>>>> the header file include/linux/printk.h needs be included. >>>>> >>>>> In include/linux/printk.h, pr_fmt is defined. >>>> This doesn't make sense, printk.h has: >>>> >>>> #ifndef pr_fmt >>>> #define pr_fmt(fmt) fmt >>>> #endif >>>> >>>> Before or after printk.h should not have an impact. >> >> >> Sorry. The previous mail is not sent successfully. I resend it. >> >>> #ifndef pr_fmt >>> >>> ... >>> >>> #endif >>> >>> The above will not undefine pr_fmt. >>> >>> #undef pr_fmt will undefine pr_fmt. >>> >>> This link explains the above in details. >>> >> https://www.techonthenet.com/c_language/directives/ifndef.php > > Why would you want to undefine it? The point is to #define it to the > rxe specific value. If it is already set to the rxe specific value > before including printk.h then it will work fine? Got your point. There are about more 130 header files that define pr_fmt in the kernel include directory. And these header files are included one another. Is there any tool to clarify the including relationship between these header files? Thanks a lot Zhu Yanjun > > Jason
On Sat, Apr 06, 2024 at 06:35:21PM +0200, Zhu Yanjun wrote: > Got your point. There are about more 130 header files that define pr_fmt in > the kernel include directory. And these header files are included one > another. Is there any tool to clarify the including relationship between > these header files? No tool I am aware of, but if headers define it then they should be local to a subsystem and never included outside it. Such as what rxe is doing. Ideally they would be private headers not under include/ Jason
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h index d8fb2c7af30a..dc2d8dd2f681 100644 --- a/drivers/infiniband/sw/rxe/rxe.h +++ b/drivers/infiniband/sw/rxe/rxe.h @@ -7,11 +7,6 @@ #ifndef RXE_H #define RXE_H -#ifdef pr_fmt -#undef pr_fmt -#endif -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - #include <linux/skbuff.h> #include <rdma/ib_verbs.h> @@ -30,6 +25,11 @@ #include "rxe_verbs.h" #include "rxe_loc.h" +#ifdef pr_fmt +#undef pr_fmt +#endif +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + /* * Version 1 and Version 2 are identical on 64 bit machines, but on 32 bit * machines Version 2 has a different struct layout.