[PATCH] cifsd: use kfree to free memory allocated by kzalloc

[PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Muhammad Usama Anjum]

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 <musamaanjum@gmail.com>
---
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);
 	}
 }
 
-- 
2.25.1

Re: [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Dan Carpenter]

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 <musamaanjum@gmail.com>
> ---
> 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.

I think that cifsd is being re-written though so it might not be worth
it.

regards,
dan carpenter

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Namjae Jeon]

2021-04-01 20:50 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
> 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 <musamaanjum@gmail.com>
>> ---
>> 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.
Thanks!
>
> regards,
> dan carpenter
>
>
> _______________________________________________
> Linux-cifsd-devel mailing list
> Linux-cifsd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-cifsd-devel
>

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 1506 bytes --]

Am 4/1/21 um 2:43 PM schrieb Namjae Jeon:
> 2021-04-01 20:50 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
>> 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 <musamaanjum@gmail.com>
>>> ---
>>> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Namjae Jeon]

2021-04-01 21:50 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 4/1/21 um 2:43 PM schrieb Namjae Jeon:
>> 2021-04-01 20:50 GMT+09:00, Dan Carpenter <dan.carpenter@oracle.com>:
>>> 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 <musamaanjum@gmail.com>
>>>> ---
>>>> 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
>
>

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Ralph Boehme]

[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]

Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <slow@samba.org>:
>> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Namjae Jeon]

2021-04-01 22:14 GMT+09:00, Ralph Boehme <slow@samba.org>:
> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <slow@samba.org>:
>>> 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
>
>

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[Tom Talpey]

On 4/1/2021 9:36 AM, Namjae Jeon wrote:
> 2021-04-01 22:14 GMT+09:00, Ralph Boehme <slow@samba.org>:
>> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
>>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <slow@samba.org>:
>>>> 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.

Re: [Linux-cifsd-devel] [PATCH] cifsd: use kfree to free memory allocated by kzalloc

[ronnie sahlberg]

On Fri, Apr 2, 2021 at 7:04 AM Tom Talpey <tom@talpey.com> wrote:
>
> On 4/1/2021 9:36 AM, Namjae Jeon wrote:
> > 2021-04-01 22:14 GMT+09:00, Ralph Boehme <slow@samba.org>:
> >> Am 4/1/21 um 2:59 PM schrieb Namjae Jeon:
> >>> 2021-04-01 21:50 GMT+09:00, Ralph Boehme <slow@samba.org>:
> >>>> 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.