Re: [PATCH 1/2] CIFS: Propagate min offload along with other parameters from primary to secondary channels.

[Paulo Alcantara <pc@manguebit.com>]

aman1cifs@gmail.com writes:

> From: Aman <aman1@microsoft.com>
>
> In a multichannel setup, it was observed that a few fields were not being
> copied over to the secondary channels, which impacted performance in cases
> where these options were relevant but not properly synchronized. To address
> this, this patch introduces copying the following parameters from the
> primary channel to the secondary channels:
>
> - min_offload
> - compression.requested
> - dfs_conn
> - ignore_signature
> - leaf_fullpath
> - noblockcnt
> - retrans
> - sign
>
> By copying these parameters, we ensure consistency across channels and
> prevent performance degradation due to missing or outdated settings.
>
> Signed-off-by: Aman <aman1@microsoft.com>
> ---
>  fs/smb/client/connect.c |  1 +
>  fs/smb/client/sess.c    | 10 ++++++++++
>  2 files changed, 11 insertions(+)
>
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index eaa6be445..eb82458eb 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -1721,6 +1721,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
>  	/* Grab netns reference for this server. */
>  	cifs_set_net_ns(tcp_ses, get_net(current->nsproxy->net_ns));
>  
> +	tcp_ses->sign = ctx->sign;
>  	tcp_ses->conn_id = atomic_inc_return(&tcpSesNextId);
>  	tcp_ses->noblockcnt = ctx->rootfs;
>  	tcp_ses->noblocksnd = ctx->noblocksnd || ctx->rootfs;
> diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
> index 91d4d409c..fdbd32a13 100644
> --- a/fs/smb/client/sess.c
> +++ b/fs/smb/client/sess.c
> @@ -522,6 +522,16 @@ cifs_ses_add_channel(struct cifs_ses *ses,
>  	ctx->sockopt_tcp_nodelay = ses->server->tcp_nodelay;
>  	ctx->echo_interval = ses->server->echo_interval / HZ;
>  	ctx->max_credits = ses->server->max_credits;
> +	ctx->min_offload = ses->server->min_offload;
> +	ctx->compress = ses->server->compression.requested;
> +	ctx->dfs_conn = ses->server->dfs_conn;
> +	ctx->ignore_signature = ses->server->ignore_signature;
> +
> +	if (ses->server->leaf_fullpath)
> +		ctx->leaf_fullpath = kstrdup(ses->server->leaf_fullpath, GFP_KERNEL);

You're missing NULL check in case kstrdup() fails and also leaking
@ctx->leaf_fullpath.

You don't need to kstrdup() it as cifs_get_tcp_session() will already do it.

Simply assign @ctx->leaf_fullpath to @ses->server->leaf_fullpath before
calling cifs_get_tcp_session().