mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] xprtrdma: removed unnecessary headers from verbs.c
@ 2023-12-26 21:23 Tanzir Hasan
  2023-12-26 23:20 ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Tanzir Hasan @ 2023-12-26 21:23 UTC (permalink / raw)
  To: Chuck Lever, Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, Trond Myklebust, Anna Schumaker, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: linux-nfs, netdev, linux-kernel, Nick Desaulniers, Al Viro, Tanzir Hasan

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>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
  2023-12-26 21:23 [PATCH] xprtrdma: removed unnecessary headers from verbs.c Tanzir Hasan
@ 2023-12-26 23:20 ` Randy Dunlap
  2023-12-26 23:35   ` Tanzir Hasan
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2023-12-26 23:20 UTC (permalink / raw)
  To: Tanzir Hasan, Chuck Lever, Jeff Layton, Neil Brown,
	Olga Kornievskaia, Dai Ngo, Tom Talpey, Trond Myklebust,
	Anna Schumaker, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: linux-nfs, netdev, linux-kernel, Nick Desaulniers, Al Viro

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
  2023-12-26 23:20 ` Randy Dunlap
@ 2023-12-26 23:35   ` Tanzir Hasan
  2023-12-26 23:53     ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Tanzir Hasan @ 2023-12-26 23:35 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Chuck Lever, Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, Trond Myklebust, Anna Schumaker, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-nfs, netdev,
	linux-kernel, Nick Desaulniers, Al Viro

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
  2023-12-26 23:35   ` Tanzir Hasan
@ 2023-12-26 23:53     ` Randy Dunlap
  2023-12-27  0:04       ` Tanzir Hasan
  0 siblings, 1 reply; 6+ messages in thread
From: Randy Dunlap @ 2023-12-26 23:53 UTC (permalink / raw)
  To: Tanzir Hasan
  Cc: Chuck Lever, Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, Trond Myklebust, Anna Schumaker, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-nfs, netdev,
	linux-kernel, Nick Desaulniers, Al Viro

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
  2023-12-26 23:53     ` Randy Dunlap
@ 2023-12-27  0:04       ` Tanzir Hasan
  2023-12-27  0:10         ` Randy Dunlap
  0 siblings, 1 reply; 6+ messages in thread
From: Tanzir Hasan @ 2023-12-27  0:04 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Chuck Lever, Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, Trond Myklebust, Anna Schumaker, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-nfs, netdev,
	linux-kernel, Nick Desaulniers, Al Viro

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xprtrdma: removed unnecessary headers from verbs.c
  2023-12-27  0:04       ` Tanzir Hasan
@ 2023-12-27  0:10         ` Randy Dunlap
  0 siblings, 0 replies; 6+ messages in thread
From: Randy Dunlap @ 2023-12-27  0:10 UTC (permalink / raw)
  To: Tanzir Hasan
  Cc: Chuck Lever, Jeff Layton, Neil Brown, Olga Kornievskaia, Dai Ngo,
	Tom Talpey, Trond Myklebust, Anna Schumaker, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-nfs, netdev,
	linux-kernel, Nick Desaulniers, Al Viro



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-27  0:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-26 21:23 [PATCH] xprtrdma: removed unnecessary headers from verbs.c Tanzir Hasan
2023-12-26 23:20 ` Randy Dunlap
2023-12-26 23:35   ` Tanzir Hasan
2023-12-26 23:53     ` Randy Dunlap
2023-12-27  0:04       ` Tanzir Hasan
2023-12-27  0:10         ` Randy Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®