Am 4/1/21 um 2:43 PM schrieb Namjae Jeon:
> 2021-04-01 20:50 GMT+09:00, Dan Carpenter <[email protected]>:
>> On Thu, Apr 01, 2021 at 04:39:33PM +0500, Muhammad Usama Anjum wrote:
>>> kfree should be used to free memory allocated by kzalloc to avoid
>>> any overhead and for maintaining consistency.
>>>
>>> Fixes: 5dfeb6d945 ("cifsd: use kmalloc() for small allocations")
>>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>>> ---
>>> This one place was left in earlier patch. I've already received
>>> responsse on that patch. I'm sending a separate patch.
>>>
>>> fs/cifsd/transport_tcp.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/cifsd/transport_tcp.c b/fs/cifsd/transport_tcp.c
>>> index 67163efcf472..040881893417 100644
>>> --- a/fs/cifsd/transport_tcp.c
>>> +++ b/fs/cifsd/transport_tcp.c
>>> @@ -551,7 +551,7 @@ void ksmbd_tcp_destroy(void)
>>> list_for_each_entry_safe(iface, tmp, &iface_list, entry) {
>>> list_del(&iface->entry);
>>> kfree(iface->name);
>>> - ksmbd_free(iface);
>>> + kfree(iface);
>>
>> We should just delete the ksmbd_free() function completely.
> Yes, I have added your review comment about this to my todo-list.
> I will do that.
>>
>> I think that cifsd is being re-written though so it might not be worth
>> it.
> Right.
fwiw, while at it what about renaming everything that still references
"cifs" to "smb" ? This is not the 90's... :)
Cheers!
-slow
2021-04-01 21:50 GMT+09:00, Ralph Boehme <[email protected]>:
> Am 4/1/21 um 2:43 PM schrieb Namjae Jeon:
>> 2021-04-01 20:50 GMT+09:00, Dan Carpenter <[email protected]>:
>>> On Thu, Apr 01, 2021 at 04:39:33PM +0500, Muhammad Usama Anjum wrote:
>>>> kfree should be used to free memory allocated by kzalloc to avoid
>>>> any overhead and for maintaining consistency.
>>>>
>>>> Fixes: 5dfeb6d945 ("cifsd: use kmalloc() for small allocations")
>>>> Signed-off-by: Muhammad Usama Anjum <[email protected]>
>>>> ---
>>>> This one place was left in earlier patch. I've already received
>>>> responsse on that patch. I'm sending a separate patch.
>>>>
>>>> fs/cifsd/transport_tcp.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/cifsd/transport_tcp.c b/fs/cifsd/transport_tcp.c
>>>> index 67163efcf472..040881893417 100644
>>>> --- a/fs/cifsd/transport_tcp.c
>>>> +++ b/fs/cifsd/transport_tcp.c
>>>> @@ -551,7 +551,7 @@ void ksmbd_tcp_destroy(void)
>>>> list_for_each_entry_safe(iface, tmp, &iface_list, entry) {
>>>> list_del(&iface->entry);
>>>> kfree(iface->name);
>>>> - ksmbd_free(iface);
>>>> + kfree(iface);
>>>
>>> We should just delete the ksmbd_free() function completely.
>> Yes, I have added your review comment about this to my todo-list.
>> I will do that.
>>>
>>> I think that cifsd is being re-written though so it might not be worth
>>> it.
>> Right.
>
> fwiw, while at it what about renaming everything that still references
> "cifs" to "smb" ? This is not the 90's... :)
It is also used with the name "ksmbd". So function and variable prefix
are used with ksmbd.
Thanks!
>
> Cheers!
> -slow
>
>
Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <[email protected]>:
>> fwiw, while at it what about renaming everything that still references
>> "cifs" to "smb" ? This is not the 90's... :)
> It is also used with the name "ksmbd". So function and variable prefix
> are used with ksmbd.
well, I was thinking of this:
> +++ b/fs/cifsd/...
We should really stop using the name cifs for modern implementation of
SMB{23} and the code should not be added as fs/cifsd/ to the kernel.
Cheers!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG-Fingerprint FAE2C6088A24252051C559E4AA1E9B7126399E46
2021-04-01 22:14 GMT+09:00, Ralph Boehme <[email protected]>:
> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <[email protected]>:
>>> fwiw, while at it what about renaming everything that still references
>>> "cifs" to "smb" ? This is not the 90's... :)
>> It is also used with the name "ksmbd". So function and variable prefix
>> are used with ksmbd.
>
> well, I was thinking of this:
>
> > +++ b/fs/cifsd/...
>
> We should really stop using the name cifs for modern implementation of
> SMB{23} and the code should not be added as fs/cifsd/ to the kernel.
As I know, currently "cifs" is being used for the subdirectory name
for historical reasons and to avoid confusions, even though the CIFS
(SMB1) dialect is no longer recommended.
>
> Cheers!
> -slow
>
> --
> Ralph Boehme, Samba Team https://samba.org/
> Samba Developer, SerNet GmbH https://sernet.de/en/samba/
> GPG-Fingerprint FAE2C6088A24252051C559E4AA1E9B7126399E46
>
>
On 4/1/2021 9:36 AM, Namjae Jeon wrote:
> 2021-04-01 22:14 GMT+09:00, Ralph Boehme <[email protected]>:
>> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
>>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <[email protected]>:
>>>> fwiw, while at it what about renaming everything that still references
>>>> "cifs" to "smb" ? This is not the 90's... :)
>>> It is also used with the name "ksmbd". So function and variable prefix
>>> are used with ksmbd.
>>
>> well, I was thinking of this:
>>
>> > +++ b/fs/cifsd/...
>>
>> We should really stop using the name cifs for modern implementation of
>> SMB{23} and the code should not be added as fs/cifsd/ to the kernel.
> As I know, currently "cifs" is being used for the subdirectory name
> for historical reasons and to avoid confusions, even though the CIFS
> (SMB1) dialect is no longer recommended.
I'm with Ralph. CIFS is history that we need to relegate to the past.
I also agree that wrappers around core memory allocators are to
be avoided.
Tom.
On Fri, Apr 2, 2021 at 7:04 AM Tom Talpey <[email protected]> wrote:
>
> On 4/1/2021 9:36 AM, Namjae Jeon wrote:
> > 2021-04-01 22:14 GMT+09:00, Ralph Boehme <[email protected]>:
> >> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
> >>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <[email protected]>:
> >>>> fwiw, while at it what about renaming everything that still references
> >>>> "cifs" to "smb" ? This is not the 90's... :)
> >>> It is also used with the name "ksmbd". So function and variable prefix
> >>> are used with ksmbd.
> >>
> >> well, I was thinking of this:
> >>
> >> > +++ b/fs/cifsd/...
> >>
> >> We should really stop using the name cifs for modern implementation of
> >> SMB{23} and the code should not be added as fs/cifsd/ to the kernel.
> > As I know, currently "cifs" is being used for the subdirectory name
> > for historical reasons and to avoid confusions, even though the CIFS
> > (SMB1) dialect is no longer recommended.
>
> I'm with Ralph. CIFS is history that we need to relegate to the past.
Tom, and Ralph.
Some background on the cifsd directory name:
We discussed in length but we decided with cifsd to align with the
current directory name cifs for the client.
Just to align with current praxis defined by other filesystems, i.e.
nfs. which has nfs for client, nfsd for server
and nfs_common for shared cod and definitions.
Once cifsd lands in the kernel I expect we will start building
cifs_common for this purpose.
An alternative would have been to rename the current fs/cifs tree to
fs/ksmb but renaming an entire directory tree
felt it might get pushback.
In the end we thought that the module name, that is user visible and
there it is important we call it smb3 something
but the source tree is not end-user visible so it was less important
what the name was.
(the alternative ending up with fs/cifs fs/ksmbd and fs/cifs_common
would have been terrible)
regards
ronnie sahlberg
>
> I also agree that wrappers around core memory allocators are to
> be avoided.
>
> Tom.