Re: [PATCH] smb: smbdirect: move fs/smb/common/smbdirect/ to fs/smb/smbdirect/

Re: [PATCH] smb: smbdirect: move fs/smb/common/smbdirect/ to fs/smb/smbdirect/

[Christoph Hellwig]

> diff --git a/fs/smb/Makefile b/fs/smb/Makefile
> index 9a1bf59a1a65..353b1c2eefc4 100644
> --- a/fs/smb/Makefile
> +++ b/fs/smb/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  obj-$(CONFIG_SMBFS)		+= common/
> +obj-$(CONFIG_SMBDIRECT)		+= smbdirect/

Why is this not in net/smbdirect/ or driver/infiniband/ulp/smdirect?

As far as I can tell there is zero file system logic in this code.

> -#include "../common/smbdirect/smbdirect_public.h"
> +#include "../smbdirect/public.h"

And all these relative includes suggest you really want a
include/linux/smdirect/ instead.

While we're at it: __SMBDIRECT_EXPORT_SYMBOL__ is really odd.
One thing is the __ pre- and postfix that make it look weird.

The other is that EXPORT_SYMBOL_FOR_MODULES is for very specific
symbols that really should not exported.  What this warrants instead
is a normal EXPORT_SYMBOL_NS_GPL.

Re: [PATCH] smb: smbdirect: move fs/smb/common/smbdirect/ to fs/smb/smbdirect/

[Stefan Metzmacher]

Hi Christoph,

>> diff --git a/fs/smb/Makefile b/fs/smb/Makefile
>> index 9a1bf59a1a65..353b1c2eefc4 100644
>> --- a/fs/smb/Makefile
>> +++ b/fs/smb/Makefile
>> @@ -1,5 +1,6 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>   
>>   obj-$(CONFIG_SMBFS)		+= common/
>> +obj-$(CONFIG_SMBDIRECT)		+= smbdirect/
> 
> Why is this not in net/smbdirect/ or driver/infiniband/ulp/smdirect?

Yes, I also thought about net/smbdirect.

As IPPROTO_SMBDIRECT or PF_SMBDIRECT will be the next step,
see the open discussion here:
https://lore.kernel.org/linux-cifs/cover.1775571957.git.metze@samba.org/
(I'll follow with that discussion soon)

I was just unsure about the consequences, e.g. would
the maintainer/pull request flow have to change in that case?
Or would Steve be able to take the changes via his trees?
Any I also didn't want to offend anybody, so I just took
what Linus proposed.

Using driver/infiniband/ulp/smdirect would also work,
if everybody prefer that.

> As far as I can tell there is zero file system logic in this code.
> 
>> -#include "../common/smbdirect/smbdirect_public.h"
>> +#include "../smbdirect/public.h"
> 
> And all these relative includes suggest you really want a
> include/linux/smdirect/ instead.

Yes, that's my also my goal in the next steps.

> While we're at it: __SMBDIRECT_EXPORT_SYMBOL__ is really odd.
> One thing is the __ pre- and postfix that make it look weird.

Yes, the __SMBDIRECT_EXPORT_SYMBOL__ was mainly a temporary
thing, now it's useless and I'll remove it.

> The other is that EXPORT_SYMBOL_FOR_MODULES is for very specific
> symbols that really should not exported.  What this warrants instead
> is a normal EXPORT_SYMBOL_NS_GPL.

I want the exported functions be minimal, as most of
of should go via the socket layer instead.

If EXPORT_SYMBOL_NS_GPL(func, "smbdirect") is better than
EXPORT_SYMBOL_FOR_MODULES() I can change that.

It means cifs.ko and ksmbd.ko would need MODULE_IMPORT_NS("smbdirect"), correct?

Thanks!
metze