[Cluster-devel] [PATCH] dlm: use sock_create_lite inside tcp_accept_from_sock

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,


On 07/08/17 20:10, Bob Peterson wrote:
> | | Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> | | ---
> | |  fs/dlm/lowcomms.c | 2 +-
> | |  1 file changed, 1 insertion(+), 1 deletion(-)
> | |
> | | diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> | | index 9382db9..4813d0e 100644
> | | --- a/fs/dlm/lowcomms.c
> | | +++ b/fs/dlm/lowcomms.c
> | | @@ -729,7 +729,7 @@ static int tcp_accept_from_sock(struct connection *con)
> | |  	mutex_unlock(&connections_lock);
> | |
> | |  	memset(&peeraddr, 0, sizeof(peeraddr));
> | | -	result = sock_create_kern(&init_net, dlm_local_addr[0]->ss_family,
> | | +	result = sock_create_lite(dlm_local_addr[0]->ss_family,
> | |  				  SOCK_STREAM, IPPROTO_TCP, &newsock);
> | |  	if (result < 0)
> | |  		return -ENOMEM;
> |
> | Isn't this also a problem for the sctp equivalent, sctp_connect_to_sock?
> |
> | Regards,
> |
> | Bob Peterson
> | Red Hat File Systems
> |
>
> In fact, I see 5 different calls to sock_create_kern in DLM.
> Shouldn't it be done to all of them?
>
> One could also argue that sock_create_kern should itself be fixed,
> not its callers.
The two functions do different things. Normally you want to create both 
a struct socket and a struct sock, which is what sock_create_kern does. 
In accept though, you need to pass in a struct socket, and the struct 
sock is created during accept and grafted on to it. That is why you were 
having such trouble with the callbacks, because it was leaking the 
struct sock that was originally created by sock_create_kern. Since DLM 
usually doesn't make many connections, that would not have shown up in 
any greatly increased memory consumption at any stage.

If you look at sctp, it calls kernel_accept() which uses 
sock_create_lite anyway, so that is already correct. We should probably 
be using that helper for the tcp case too, which would be perhaps a 
better way to resolve that problem, since it reduces code duplication,

Steve.

> Regards,
>
> Bob Peterson
> Red Hat File Systems
>