[PATCH] xprtrdma: removed unnecessary headers from verbs.c

Tanzir Hasan posted 1 patch 2 years ago
net/sunrpc/xprtrdma/verbs.c | 3 ---
1 file changed, 3 deletions(-)
[PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Tanzir Hasan 2 years ago
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(-)

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,
-- 
Tanzir Hasan <tanzirh@google.com>
Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Randy Dunlap 2 years ago
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,

-- 
#Randy
Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Tanzir Hasan 1 year, 12 months ago
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
Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Randy Dunlap 1 year, 12 months ago
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.

-- 
#Randy
Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Tanzir Hasan 1 year, 12 months ago
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
Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
Posted by Randy Dunlap 1 year, 12 months ago

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.

-- 
#Randy