| Message ID | 20231226-verbs-v1-1-3a2cecf11afd@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xprtrdma: removed unnecessary headers from verbs.c | expand |
Hi, On 12/26/23 13:23, Tanzir Hasan wrote: > asm-generic/barrier.h and asm/bitops.h are already brought into the > header and the file can still be built with their removal. Brought into which header? Does this conflict with Rule #1 in Documentation/process/submit-checklist.rst ? > Suggested-by: Al Viro <viro@zeniv.linux.org.uk> > Signed-off-by: Tanzir Hasan <tanzirh@google.com> > --- > net/sunrpc/xprtrdma/verbs.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c > index 28c0771c4e8c..5436560dda85 100644 > --- a/net/sunrpc/xprtrdma/verbs.c > +++ b/net/sunrpc/xprtrdma/verbs.c > @@ -55,9 +55,6 @@ > #include <linux/sunrpc/svc_rdma.h> > #include <linux/log2.h> > > -#include <asm-generic/barrier.h> > -#include <asm/bitops.h> > - > #include <rdma/ib_cm.h> > > #include "xprt_rdma.h" > > --- > base-commit: fbafc3e621c3f4ded43720fdb1d6ce1728ec664e > change-id: 20231226-verbs-30800631d3f1 > > Best regards,
On Tue, Dec 26, 2023 at 3:20 PM Randy Dunlap <rdunlap@infradead.org> wrote: > > Hi, > > On 12/26/23 13:23, Tanzir Hasan wrote: > > asm-generic/barrier.h and asm/bitops.h are already brought into the > > header and the file can still be built with their removal. > > Brought into which header? Hi Randy, Sorry for the poor explanation. I see that I left out the specific header. The inclusion of linux/sunrpc/svc_rdma.h brings in linux/sunrpc/rpc_rdma.h This brings in linux/bitops.h which is preferred over asm/bitops.h > Does this conflict with Rule #1 in Documentation/process/submit-checklist.rst ? Yes, this conflicts with Rule #1. A better version of this patch would be to add linux/bitops.h to this file directly. The main reason this patch exists is to clear out the asm-generic file since those are not preferred. I can do this by either including just linux/bitops.h or including both linux/bitops.h and asm/barrier.h. Would the second approach conform better with Rule #1? Thanks, Tanzir
Hi Tanzir, On 12/26/23 15:35, Tanzir Hasan wrote: > On Tue, Dec 26, 2023 at 3:20 PM Randy Dunlap <rdunlap@infradead.org> wrote: >> >> Hi, >> >> On 12/26/23 13:23, Tanzir Hasan wrote: >>> asm-generic/barrier.h and asm/bitops.h are already brought into the >>> header and the file can still be built with their removal. >> >> Brought into which header? > Hi Randy, > > Sorry for the poor explanation. I see that I left out the specific header. > The inclusion of linux/sunrpc/svc_rdma.h brings in linux/sunrpc/rpc_rdma.h > This brings in linux/bitops.h which is preferred over asm/bitops.h > >> Does this conflict with Rule #1 in Documentation/process/submit-checklist.rst ? > > Yes, this conflicts with Rule #1. A better version of this patch would be to add > linux/bitops.h to this file directly. The main reason this patch > exists is to clear > out the asm-generic file since those are not preferred. I can do this by either > including just linux/bitops.h or including both linux/bitops.h and > asm/barrier.h. > Would the second approach conform better with Rule #1? Yes, it would IMO. Where can I find your current working list of what/how to #include? Thanks.
Hi Randy,
> Where can I find your current working list of what/how to #include?
Here is my current working list of what to #include.
#include <linux/bitops.h>
#include <linux/interrupt.h>
#include <linux/slab.h>
#include <linux/sunrpc/addr.h>
#include <linux/sunrpc/svc_rdma.h>
#include <linux/log2.h>
#include <asm/barrier.h>
#include <rdma/ib_cm.h>
#include "xprt_rdma.h"
#include <trace/events/rpcrdma.h>
There was a discussion here about when to include asm/asm-generics:
https://lore.kernel.org/llvm/20231215210344.GA3096493@ZenIV/
If I misunderstood your question please let me know.
Best,
Tanzir
On 12/26/23 16:04, Tanzir Hasan wrote: > Hi Randy, > >> Where can I find your current working list of what/how to #include? > Here is my current working list of what to #include. > > #include <linux/bitops.h> > #include <linux/interrupt.h> > #include <linux/slab.h> > #include <linux/sunrpc/addr.h> > #include <linux/sunrpc/svc_rdma.h> > #include <linux/log2.h> > > #include <asm/barrier.h> > > #include <rdma/ib_cm.h> > > #include "xprt_rdma.h" > #include <trace/events/rpcrdma.h> > > There was a discussion here about when to include asm/asm-generics: > https://lore.kernel.org/llvm/20231215210344.GA3096493@ZenIV/ > > If I misunderstood your question please let me know. Yes, more the latter link for general info rather than the specific info for this one source file. Thanks.
diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c index 28c0771c4e8c..5436560dda85 100644 --- a/net/sunrpc/xprtrdma/verbs.c +++ b/net/sunrpc/xprtrdma/verbs.c @@ -55,9 +55,6 @@ #include <linux/sunrpc/svc_rdma.h> #include <linux/log2.h> -#include <asm-generic/barrier.h> -#include <asm/bitops.h> - #include <rdma/ib_cm.h> #include "xprt_rdma.h"
asm-generic/barrier.h and asm/bitops.h are already brought into the header and the file can still be built with their removal. Suggested-by: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: Tanzir Hasan <tanzirh@google.com> --- net/sunrpc/xprtrdma/verbs.c | 3 --- 1 file changed, 3 deletions(-) --- base-commit: fbafc3e621c3f4ded43720fdb1d6ce1728ec664e change-id: 20231226-verbs-30800631d3f1 Best regards,