| Message ID | 20210401113933.GA2828895@LEGION |
|---|---|
| State | New |
| Headers | show |
| Series | cifsd: use kfree to free memory allocated by kzalloc | expand |
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 >
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 > >
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.
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.
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); } }
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(-)