| Message ID | 20171219101327.2165797-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Arnd, > diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig > index 500fd69fb58b..3bfc55c08bef 100644 > --- a/fs/cifs/Kconfig > +++ b/fs/cifs/Kconfig > @@ -199,6 +199,7 @@ config CIFS_SMB311 > config CIFS_SMB_DIRECT > bool "SMB Direct support (Experimental)" > depends on CIFS && INFINIBAND > + depends on CIFS=m || INFINIBAND=y > help > Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1. > SMB Direct allows transferring SMB packets over RDMA. If unsure, Is this really correct? Should CIFS_SMB_DIRECT be allowed with: CIFS=n and INFINIBAND=y ??? or CIFS=m and INFINIBAND=n ??? I guess a more complex logic should be used here or am I missing something? metze
On Tue, Dec 19, 2017 at 11:33 AM, Stefan Metzmacher <metze@samba.org> wrote: > Hi Arnd, > >> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig >> index 500fd69fb58b..3bfc55c08bef 100644 >> --- a/fs/cifs/Kconfig >> +++ b/fs/cifs/Kconfig >> @@ -199,6 +199,7 @@ config CIFS_SMB311 >> config CIFS_SMB_DIRECT >> bool "SMB Direct support (Experimental)" >> depends on CIFS && INFINIBAND >> + depends on CIFS=m || INFINIBAND=y >> help >> Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1. >> SMB Direct allows transferring SMB packets over RDMA. If unsure, > > Is this really correct? Should CIFS_SMB_DIRECT be allowed with: > > CIFS=n and INFINIBAND=y ??? > or > CIFS=m and INFINIBAND=n ??? > > I guess a more complex logic should be used here > or am I missing something? The two ones you listed are prohibited by the existing 'depends on CIFS && INFINIBAND' dependency. We could rephrase the dependency as depends on (CIFS=y && INFINIBAND=y) || \ (CIFS=m && INFINIBAND=y) || \ (CIFS=m && INFINIBAND=m) which has the same effect as depends on CIFS && INFINIBAND depends on CIFS=m || INFINIBAND=y but I don't think that adds any clarity. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 19.12.2017 um 11:56 schrieb Arnd Bergmann via samba-technical: > On Tue, Dec 19, 2017 at 11:33 AM, Stefan Metzmacher <metze@samba.org> wrote: >> Hi Arnd, >> >>> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig >>> index 500fd69fb58b..3bfc55c08bef 100644 >>> --- a/fs/cifs/Kconfig >>> +++ b/fs/cifs/Kconfig >>> @@ -199,6 +199,7 @@ config CIFS_SMB311 >>> config CIFS_SMB_DIRECT >>> bool "SMB Direct support (Experimental)" >>> depends on CIFS && INFINIBAND >>> + depends on CIFS=m || INFINIBAND=y >>> help >>> Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1. >>> SMB Direct allows transferring SMB packets over RDMA. If unsure, >> >> Is this really correct? Should CIFS_SMB_DIRECT be allowed with: >> >> CIFS=n and INFINIBAND=y ??? >> or >> CIFS=m and INFINIBAND=n ??? >> >> I guess a more complex logic should be used here >> or am I missing something? > > The two ones you listed are prohibited by the existing > 'depends on CIFS && INFINIBAND' dependency. > > We could rephrase the dependency as > > depends on (CIFS=y && INFINIBAND=y) || \ > (CIFS=m && INFINIBAND=y) || \ > (CIFS=m && INFINIBAND=m) > > which has the same effect as > > depends on CIFS && INFINIBAND > depends on CIFS=m || INFINIBAND=y > > but I don't think that adds any clarity. Thanks for the clarification! I somehow assumed the patch has been: - depends on CIFS && INFINIBAND + depends on CIFS=m || INFINIBAND=y instead of: depends on CIFS && INFINIBAND + depends on CIFS=m || INFINIBAND=y metze
On Tue, 19 Dec 2017 11:12:57 +0100, Arnd Bergmann wrote: > A built-in SMB file system cannot link against a modular > infiniband core module: > > fs/cifs/smbdirect.o: In function `smbd_destroy_rdma_work': > smbdirect.c:(.text+0x28e3): undefined reference to `ib_drain_qp' > smbdirect.c:(.text+0x2915): undefined reference to `rdma_destroy_qp' > smbdirect.c:(.text+0x2d08): undefined reference to `ib_free_cq' > smbdirect.c:(.text+0x2d3d): undefined reference to `ib_free_cq' > smbdirect.c:(.text+0x2d6f): undefined reference to `ib_dealloc_pd' > smbdirect.c:(.text+0x2d9a): undefined reference to `rdma_destroy_id' > fs/cifs/smbdirect.o: In function `smbd_disconnect_rdma_work': > smbdirect.c:(.text+0x3d3e): undefined reference to `rdma_disconnect' > fs/cifs/smbdirect.o: In function `_smbd_get_connection': > smbdirect.c:(.text+0x5bc2): undefined reference to `rdma_create_id' > smbdirect.c:(.text+0x5cc5): undefined reference to `rdma_resolve_addr' > smbdirect.c:(.text+0x5cda): undefined reference to `rdma_destroy_id' > smbdirect.c:(.text+0x5da7): undefined reference to `rdma_destroy_id' > smbdirect.c:(.text+0x5e89): undefined reference to `rdma_resolve_route' > smbdirect.c:(.text+0x5f07): undefined reference to `rdma_destroy_id' > > This changes the Kconfig dependency to enforce selecting one of the > valid configurations. > > Fixes: bbc50d2ad317 ("CIFS: SMBD: Introduce kernel config option CONFIG_CIFS_SMB_DIRECT") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > fs/cifs/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig > index 500fd69fb58b..3bfc55c08bef 100644 > --- a/fs/cifs/Kconfig > +++ b/fs/cifs/Kconfig > @@ -199,6 +199,7 @@ config CIFS_SMB311 > config CIFS_SMB_DIRECT > bool "SMB Direct support (Experimental)" > depends on CIFS && INFINIBAND > + depends on CIFS=m || INFINIBAND=y > help > Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1. > SMB Direct allows transferring SMB packets over RDMA. If unsure, Looks good to me. Reviewed-by: Jean Delvare <jdelvare@suse.de>
> depends on CIFS && INFINIBAND > + depends on CIFS=m || INFINIBAND=y How about we change them to depends on CIFS=m && INFINIBAND || CIFS=y && INFINIBAND=y This makes it easy to read. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 19.12.2017 um 22:21 schrieb Long Li via samba-technical: >> depends on CIFS && INFINIBAND >> + depends on CIFS=m || INFINIBAND=y > > How about we change them to > > depends on CIFS=m && INFINIBAND || CIFS=y && INFINIBAND=y > > This makes it easy to read. I like it :-) metze
On Tue, Dec 19, 2017 at 10:21 PM, Long Li <longli@microsoft.com> wrote: >> depends on CIFS && INFINIBAND >> + depends on CIFS=m || INFINIBAND=y > > How about we change them to > > depends on CIFS=m && INFINIBAND || CIFS=y && INFINIBAND=y > > This makes it easy to read. Yes, that seems fine. I would normally group them using () to avoid any confusion with operator precedence, but your version is also correct, so just pick the one you like best. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig index 500fd69fb58b..3bfc55c08bef 100644 --- a/fs/cifs/Kconfig +++ b/fs/cifs/Kconfig @@ -199,6 +199,7 @@ config CIFS_SMB311 config CIFS_SMB_DIRECT bool "SMB Direct support (Experimental)" depends on CIFS && INFINIBAND + depends on CIFS=m || INFINIBAND=y help Enables SMB Direct experimental support for SMB 3.0, 3.02 and 3.1.1. SMB Direct allows transferring SMB packets over RDMA. If unsure,
A built-in SMB file system cannot link against a modular infiniband core module: fs/cifs/smbdirect.o: In function `smbd_destroy_rdma_work': smbdirect.c:(.text+0x28e3): undefined reference to `ib_drain_qp' smbdirect.c:(.text+0x2915): undefined reference to `rdma_destroy_qp' smbdirect.c:(.text+0x2d08): undefined reference to `ib_free_cq' smbdirect.c:(.text+0x2d3d): undefined reference to `ib_free_cq' smbdirect.c:(.text+0x2d6f): undefined reference to `ib_dealloc_pd' smbdirect.c:(.text+0x2d9a): undefined reference to `rdma_destroy_id' fs/cifs/smbdirect.o: In function `smbd_disconnect_rdma_work': smbdirect.c:(.text+0x3d3e): undefined reference to `rdma_disconnect' fs/cifs/smbdirect.o: In function `_smbd_get_connection': smbdirect.c:(.text+0x5bc2): undefined reference to `rdma_create_id' smbdirect.c:(.text+0x5cc5): undefined reference to `rdma_resolve_addr' smbdirect.c:(.text+0x5cda): undefined reference to `rdma_destroy_id' smbdirect.c:(.text+0x5da7): undefined reference to `rdma_destroy_id' smbdirect.c:(.text+0x5e89): undefined reference to `rdma_resolve_route' smbdirect.c:(.text+0x5f07): undefined reference to `rdma_destroy_id' This changes the Kconfig dependency to enforce selecting one of the valid configurations. Fixes: bbc50d2ad317 ("CIFS: SMBD: Introduce kernel config option CONFIG_CIFS_SMB_DIRECT") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- fs/cifs/Kconfig | 1 + 1 file changed, 1 insertion(+)