diff mbox

[v2,4/4] CIFS: Migrate to shared superblock model

Message ID 1304589314-1147-4-git-send-email-piastry@etersoft.ru (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Shilovsky May 5, 2011, 9:55 a.m. UTC
Add cifs_match_super to use in sget to share superblock between mounts
that have the same //server/sharename, credentials and mount options.
It helps us to improve performance on work with future SMB2.1 leases.

Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
---
 fs/cifs/cifsfs.c   |   18 ++++++++++-
 fs/cifs/cifsglob.h |    5 +++
 fs/cifs/connect.c  |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 1 deletions(-)

Comments

Jeff Layton May 24, 2011, 11:22 a.m. UTC | #1
On Thu,  5 May 2011 13:55:14 +0400
Pavel Shilovsky <piastry@etersoft.ru> wrote:

> Add cifs_match_super to use in sget to share superblock between mounts
> that have the same //server/sharename, credentials and mount options.
> It helps us to improve performance on work with future SMB2.1 leases.
> 
> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
> ---
>  fs/cifs/cifsfs.c   |   18 ++++++++++-
>  fs/cifs/cifsglob.h |    5 +++
>  fs/cifs/connect.c  |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 112 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index 446b5c2..ba0b7ae 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -644,6 +644,7 @@ cifs_do_mount(struct file_system_type *fs_type,
>  	struct super_block *sb;
>  	struct cifs_sb_info *cifs_sb;
>  	struct smb_vol *volume_info;
> +	struct cifs_mnt_data mnt_data;
>  	struct dentry *root;
>  	char *copied_data = NULL;
>  
> @@ -661,13 +662,21 @@ cifs_do_mount(struct file_system_type *fs_type,
>  
>  	cifs_setup_cifs_sb(volume_info, cifs_sb);
>  
> -	sb = sget(fs_type, NULL, set_anon_super, NULL);
> +	mnt_data.vol = volume_info;
> +	mnt_data.cifs_sb = cifs_sb;
> +
> +	sb = sget(fs_type, cifs_match_super, set_anon_super, &mnt_data);
>  	if (IS_ERR(sb)) {
>  		kfree(cifs_sb);
>  		root = ERR_CAST(sb);
>  		goto out;
>  	}
>  
> +	if (sb->s_fs_info) {
> +		cFYI(1, "Use existing superblock");
> +		goto out_shared;
> +	}
> +
>  	sb->s_flags = flags;
>  
>  	/*
> @@ -700,6 +709,13 @@ out:
>  	cifs_cleanup_volume_info(&volume_info);
>  	return root;
>  
> +out_shared:
> +	root = cifs_get_root(volume_info, sb);
> +	if (root)
> +		cFYI(1, "dentry root is: %p", root);
> +	cifs_cleanup_volume_info(&volume_info);
> +	return root;
> +
>  err_out:
>  	kfree(cifs_sb);
>  	deactivate_locked_super(sb);
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 7ad7d69..55901b0 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -211,6 +211,11 @@ struct smb_vol {
>  	struct nls_table *local_nls;
>  };
>  
> +struct cifs_mnt_data {
> +	struct cifs_sb_info *cifs_sb;
> +	struct smb_vol *vol;
> +};
> +
>  struct TCP_Server_Info {
>  	struct list_head tcp_ses_list;
>  	struct list_head smb_ses_list;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1cacfaa..1d39dcf 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2135,6 +2135,96 @@ cifs_put_tlink(struct tcon_link *tlink)
>  	return;
>  }
>  
> +static inline struct tcon_link *
> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb);
> +
> +static const u32 ACTUAL_MOUNT_FLAGS = CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
> +	CIFS_MOUNT_SERVER_INUM | CIFS_MOUNT_DIRECT_IO | CIFS_MOUNT_NO_XATTR |
> +	CIFS_MOUNT_MAP_SPECIAL_CHR | CIFS_MOUNT_UNX_EMUL | CIFS_MOUNT_NO_BRL |
> +	CIFS_MOUNT_CIFS_ACL | CIFS_MOUNT_OVERR_UID | CIFS_MOUNT_OVERR_GID |
> +	CIFS_MOUNT_DYNPERM | CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC |
> +	CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | CIFS_MOUNT_MULTIUSER |
> +	CIFS_MOUNT_STRICT_IO;
> +
> +static int
> +compare_mount_options(struct cifs_sb_info *old, struct cifs_sb_info *new)
> +{
> +	if ((old->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS) !=
> +	    (new->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS))
> +		return 0;
> +
> +	if (old->rsize != new->rsize || old->wsize != new->wsize)
> +		return 0;
> +
		^^^^^^^^^^
The rsize and wsize are negotiated -- you can't know what this is until
you talk to the server. Doesn't this check always fail?

The wsize at least is never negotiated up. We only ever negotiate down,
so what may be best is to see whether the wsize was actually specified
in the mount options and if it's smaller than the existing one. If it
is then don't match.

I plan to fix the rsize negotiation in a similar fashion once I do a
readpages overhaul.

> +	if (old->mnt_uid != new->mnt_uid || old->mnt_gid != new->mnt_gid)
> +		return 0;
> +
> +	if (old->mnt_file_mode != new->mnt_file_mode ||
> +	    old->mnt_dir_mode != new->mnt_dir_mode)
> +		return 0;
> +
> +	if (strcmp(old->local_nls->charset, new->local_nls->charset))
> +		return 0;
> +
> +	if (old->actimeo != new->actimeo)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +int
> +cifs_match_super(struct super_block *sb, void *data)
> +{
> +	struct cifs_mnt_data *mnt_data = (struct cifs_mnt_data *)data;
> +	struct smb_vol *volume_info;
> +	struct cifs_sb_info *cifs_sb, *new_cifs_sb;
> +	struct TCP_Server_Info *tcp_srv;
> +	struct cifsSesInfo *ses;
> +	struct cifsTconInfo *tcon;
> +	struct tcon_link *tlink;
> +	struct sockaddr_storage addr;
> +	int rc = 0;
> +
> +	memset(&addr, 0, sizeof(struct sockaddr_storage));
> +
> +	spin_lock(&cifs_tcp_ses_lock);
> +	cifs_sb = CIFS_SB(sb);
> +	tlink = cifs_get_tlink(cifs_sb_master_tlink(cifs_sb));
> +	if (IS_ERR(tlink)) {
> +		spin_unlock(&cifs_tcp_ses_lock);
> +		return rc;
> +	}
> +	tcon = tlink_tcon(tlink);
> +	ses = tcon->ses;
> +	tcp_srv = ses->server;
> +
> +	volume_info = mnt_data->vol;
> +	new_cifs_sb = mnt_data->cifs_sb;
> +
> +	if (!volume_info->UNCip || !volume_info->UNC)
> +		goto out;
> +
> +	rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
> +				volume_info->UNCip,
> +				strlen(volume_info->UNCip),
> +				volume_info->port);
> +	if (!rc)
> +		goto out;
> +
> +	if (!match_server(tcp_srv, (struct sockaddr *)&addr, volume_info) ||
> +	    !match_session(ses, volume_info) ||
> +	    !match_tcon(tcon, volume_info->UNC)) {
> +		rc = 0;
> +		goto out;
> +	}
> +
> +	rc = compare_mount_options(cifs_sb, new_cifs_sb);
> +out:
> +	cifs_put_tlink(tlink);
> +	spin_unlock(&cifs_tcp_ses_lock);
> +	return rc;
> +}
> +
>  int
>  get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path,
>  	     const struct nls_table *nls_codepage, unsigned int *pnum_referrals,


Other than the problem with the rsize/wsize, this looks ok. I don't see
that as a show-stopper either, so I suggest we merge this and let Pavel
fix that after the merge window closes.

Reviewed-by: Jeff Layton <jlayton@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Shilovsky May 25, 2011, 7:10 p.m. UTC | #2
2011/5/24 Jeff Layton <jlayton@redhat.com>:
> On Thu,  5 May 2011 13:55:14 +0400
> Pavel Shilovsky <piastry@etersoft.ru> wrote:
>
>> Add cifs_match_super to use in sget to share superblock between mounts
>> that have the same //server/sharename, credentials and mount options.
>> It helps us to improve performance on work with future SMB2.1 leases.
>>
>> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
>> ---
>>  fs/cifs/cifsfs.c   |   18 ++++++++++-
>>  fs/cifs/cifsglob.h |    5 +++
>>  fs/cifs/connect.c  |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 112 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index 446b5c2..ba0b7ae 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -644,6 +644,7 @@ cifs_do_mount(struct file_system_type *fs_type,
>>       struct super_block *sb;
>>       struct cifs_sb_info *cifs_sb;
>>       struct smb_vol *volume_info;
>> +     struct cifs_mnt_data mnt_data;
>>       struct dentry *root;
>>       char *copied_data = NULL;
>>
>> @@ -661,13 +662,21 @@ cifs_do_mount(struct file_system_type *fs_type,
>>
>>       cifs_setup_cifs_sb(volume_info, cifs_sb);
>>
>> -     sb = sget(fs_type, NULL, set_anon_super, NULL);
>> +     mnt_data.vol = volume_info;
>> +     mnt_data.cifs_sb = cifs_sb;
>> +
>> +     sb = sget(fs_type, cifs_match_super, set_anon_super, &mnt_data);
>>       if (IS_ERR(sb)) {
>>               kfree(cifs_sb);
>>               root = ERR_CAST(sb);
>>               goto out;
>>       }
>>
>> +     if (sb->s_fs_info) {
>> +             cFYI(1, "Use existing superblock");
>> +             goto out_shared;
>> +     }
>> +
>>       sb->s_flags = flags;
>>
>>       /*
>> @@ -700,6 +709,13 @@ out:
>>       cifs_cleanup_volume_info(&volume_info);
>>       return root;
>>
>> +out_shared:
>> +     root = cifs_get_root(volume_info, sb);
>> +     if (root)
>> +             cFYI(1, "dentry root is: %p", root);
>> +     cifs_cleanup_volume_info(&volume_info);
>> +     return root;
>> +
>>  err_out:
>>       kfree(cifs_sb);
>>       deactivate_locked_super(sb);
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 7ad7d69..55901b0 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -211,6 +211,11 @@ struct smb_vol {
>>       struct nls_table *local_nls;
>>  };
>>
>> +struct cifs_mnt_data {
>> +     struct cifs_sb_info *cifs_sb;
>> +     struct smb_vol *vol;
>> +};
>> +
>>  struct TCP_Server_Info {
>>       struct list_head tcp_ses_list;
>>       struct list_head smb_ses_list;
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 1cacfaa..1d39dcf 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2135,6 +2135,96 @@ cifs_put_tlink(struct tcon_link *tlink)
>>       return;
>>  }
>>
>> +static inline struct tcon_link *
>> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb);
>> +
>> +static const u32 ACTUAL_MOUNT_FLAGS = CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
>> +     CIFS_MOUNT_SERVER_INUM | CIFS_MOUNT_DIRECT_IO | CIFS_MOUNT_NO_XATTR |
>> +     CIFS_MOUNT_MAP_SPECIAL_CHR | CIFS_MOUNT_UNX_EMUL | CIFS_MOUNT_NO_BRL |
>> +     CIFS_MOUNT_CIFS_ACL | CIFS_MOUNT_OVERR_UID | CIFS_MOUNT_OVERR_GID |
>> +     CIFS_MOUNT_DYNPERM | CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC |
>> +     CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | CIFS_MOUNT_MULTIUSER |
>> +     CIFS_MOUNT_STRICT_IO;
>> +
>> +static int
>> +compare_mount_options(struct cifs_sb_info *old, struct cifs_sb_info *new)
>> +{
>> +     if ((old->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS) !=
>> +         (new->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS))
>> +             return 0;
>> +
>> +     if (old->rsize != new->rsize || old->wsize != new->wsize)
>> +             return 0;
>> +
>                ^^^^^^^^^^
> The rsize and wsize are negotiated -- you can't know what this is until
> you talk to the server. Doesn't this check always fail?

If we don't specify wsize and rsize they are set to default values
(16384 and 57344) in both mounts, this check succeed.

>
> The wsize at least is never negotiated up. We only ever negotiate down,
> so what may be best is to see whether the wsize was actually specified
> in the mount options and if it's smaller than the existing one. If it
> is then don't match.
>
> I plan to fix the rsize negotiation in a similar fashion once I do a
> readpages overhaul.

Ok, when it is done, we can change compare_mount_options appropriately.

>
>> +     if (old->mnt_uid != new->mnt_uid || old->mnt_gid != new->mnt_gid)
>> +             return 0;
>> +
>> +     if (old->mnt_file_mode != new->mnt_file_mode ||
>> +         old->mnt_dir_mode != new->mnt_dir_mode)
>> +             return 0;
>> +
>> +     if (strcmp(old->local_nls->charset, new->local_nls->charset))
>> +             return 0;
>> +
>> +     if (old->actimeo != new->actimeo)
>> +             return 0;
>> +
>> +     return 1;
>> +}
>> +
>> +int
>> +cifs_match_super(struct super_block *sb, void *data)
>> +{
>> +     struct cifs_mnt_data *mnt_data = (struct cifs_mnt_data *)data;
>> +     struct smb_vol *volume_info;
>> +     struct cifs_sb_info *cifs_sb, *new_cifs_sb;
>> +     struct TCP_Server_Info *tcp_srv;
>> +     struct cifsSesInfo *ses;
>> +     struct cifsTconInfo *tcon;
>> +     struct tcon_link *tlink;
>> +     struct sockaddr_storage addr;
>> +     int rc = 0;
>> +
>> +     memset(&addr, 0, sizeof(struct sockaddr_storage));
>> +
>> +     spin_lock(&cifs_tcp_ses_lock);
>> +     cifs_sb = CIFS_SB(sb);
>> +     tlink = cifs_get_tlink(cifs_sb_master_tlink(cifs_sb));
>> +     if (IS_ERR(tlink)) {
>> +             spin_unlock(&cifs_tcp_ses_lock);
>> +             return rc;
>> +     }
>> +     tcon = tlink_tcon(tlink);
>> +     ses = tcon->ses;
>> +     tcp_srv = ses->server;
>> +
>> +     volume_info = mnt_data->vol;
>> +     new_cifs_sb = mnt_data->cifs_sb;
>> +
>> +     if (!volume_info->UNCip || !volume_info->UNC)
>> +             goto out;
>> +
>> +     rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
>> +                             volume_info->UNCip,
>> +                             strlen(volume_info->UNCip),
>> +                             volume_info->port);
>> +     if (!rc)
>> +             goto out;
>> +
>> +     if (!match_server(tcp_srv, (struct sockaddr *)&addr, volume_info) ||
>> +         !match_session(ses, volume_info) ||
>> +         !match_tcon(tcon, volume_info->UNC)) {
>> +             rc = 0;
>> +             goto out;
>> +     }
>> +
>> +     rc = compare_mount_options(cifs_sb, new_cifs_sb);
>> +out:
>> +     cifs_put_tlink(tlink);
>> +     spin_unlock(&cifs_tcp_ses_lock);
>> +     return rc;
>> +}
>> +
>>  int
>>  get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path,
>>            const struct nls_table *nls_codepage, unsigned int *pnum_referrals,
>
>
> Other than the problem with the rsize/wsize, this looks ok. I don't see
> that as a show-stopper either, so I suggest we merge this and let Pavel
> fix that after the merge window closes.
>
> Reviewed-by: Jeff Layton <jlayton@redhat.com>
>
Jeff Layton May 25, 2011, 7:36 p.m. UTC | #3
On Wed, 25 May 2011 23:10:33 +0400
Pavel Shilovsky <piastry@etersoft.ru> wrote:

> 2011/5/24 Jeff Layton <jlayton@redhat.com>:
> > On Thu,  5 May 2011 13:55:14 +0400
> > Pavel Shilovsky <piastry@etersoft.ru> wrote:
> >
> >> Add cifs_match_super to use in sget to share superblock between mounts
> >> that have the same //server/sharename, credentials and mount options.
> >> It helps us to improve performance on work with future SMB2.1 leases.
> >>
> >> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
> >> ---
> >>  fs/cifs/cifsfs.c   |   18 ++++++++++-
> >>  fs/cifs/cifsglob.h |    5 +++
> >>  fs/cifs/connect.c  |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 112 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> >> index 446b5c2..ba0b7ae 100644
> >> --- a/fs/cifs/cifsfs.c
> >> +++ b/fs/cifs/cifsfs.c
> >> @@ -644,6 +644,7 @@ cifs_do_mount(struct file_system_type *fs_type,
> >>       struct super_block *sb;
> >>       struct cifs_sb_info *cifs_sb;
> >>       struct smb_vol *volume_info;
> >> +     struct cifs_mnt_data mnt_data;
> >>       struct dentry *root;
> >>       char *copied_data = NULL;
> >>
> >> @@ -661,13 +662,21 @@ cifs_do_mount(struct file_system_type *fs_type,
> >>
> >>       cifs_setup_cifs_sb(volume_info, cifs_sb);
> >>
> >> -     sb = sget(fs_type, NULL, set_anon_super, NULL);
> >> +     mnt_data.vol = volume_info;
> >> +     mnt_data.cifs_sb = cifs_sb;
> >> +
> >> +     sb = sget(fs_type, cifs_match_super, set_anon_super, &mnt_data);
> >>       if (IS_ERR(sb)) {
> >>               kfree(cifs_sb);
> >>               root = ERR_CAST(sb);
> >>               goto out;
> >>       }
> >>
> >> +     if (sb->s_fs_info) {
> >> +             cFYI(1, "Use existing superblock");
> >> +             goto out_shared;
> >> +     }
> >> +
> >>       sb->s_flags = flags;
> >>
> >>       /*
> >> @@ -700,6 +709,13 @@ out:
> >>       cifs_cleanup_volume_info(&volume_info);
> >>       return root;
> >>
> >> +out_shared:
> >> +     root = cifs_get_root(volume_info, sb);
> >> +     if (root)
> >> +             cFYI(1, "dentry root is: %p", root);
> >> +     cifs_cleanup_volume_info(&volume_info);
> >> +     return root;
> >> +
> >>  err_out:
> >>       kfree(cifs_sb);
> >>       deactivate_locked_super(sb);
> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> >> index 7ad7d69..55901b0 100644
> >> --- a/fs/cifs/cifsglob.h
> >> +++ b/fs/cifs/cifsglob.h
> >> @@ -211,6 +211,11 @@ struct smb_vol {
> >>       struct nls_table *local_nls;
> >>  };
> >>
> >> +struct cifs_mnt_data {
> >> +     struct cifs_sb_info *cifs_sb;
> >> +     struct smb_vol *vol;
> >> +};
> >> +
> >>  struct TCP_Server_Info {
> >>       struct list_head tcp_ses_list;
> >>       struct list_head smb_ses_list;
> >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> >> index 1cacfaa..1d39dcf 100644
> >> --- a/fs/cifs/connect.c
> >> +++ b/fs/cifs/connect.c
> >> @@ -2135,6 +2135,96 @@ cifs_put_tlink(struct tcon_link *tlink)
> >>       return;
> >>  }
> >>
> >> +static inline struct tcon_link *
> >> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb);
> >> +
> >> +static const u32 ACTUAL_MOUNT_FLAGS = CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
> >> +     CIFS_MOUNT_SERVER_INUM | CIFS_MOUNT_DIRECT_IO | CIFS_MOUNT_NO_XATTR |
> >> +     CIFS_MOUNT_MAP_SPECIAL_CHR | CIFS_MOUNT_UNX_EMUL | CIFS_MOUNT_NO_BRL |
> >> +     CIFS_MOUNT_CIFS_ACL | CIFS_MOUNT_OVERR_UID | CIFS_MOUNT_OVERR_GID |
> >> +     CIFS_MOUNT_DYNPERM | CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC |
> >> +     CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | CIFS_MOUNT_MULTIUSER |
> >> +     CIFS_MOUNT_STRICT_IO;
> >> +
> >> +static int
> >> +compare_mount_options(struct cifs_sb_info *old, struct cifs_sb_info *new)
> >> +{
> >> +     if ((old->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS) !=
> >> +         (new->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS))
> >> +             return 0;
> >> +
> >> +     if (old->rsize != new->rsize || old->wsize != new->wsize)
> >> +             return 0;
> >> +
> >                ^^^^^^^^^^
> > The rsize and wsize are negotiated -- you can't know what this is until
> > you talk to the server. Doesn't this check always fail?
> 
> If we don't specify wsize and rsize they are set to default values
> (16384 and 57344) in both mounts, this check succeed.
> 

Ahh then this will likely be broken if/when the wsize negotiation patch
goes in. I suppose we'll have to see which patch (if any) Steve decides
to commit first and then try to fix this up afterward.

The brokenness shouldn't be too bad, you just won't be able to actually
share superblocks until it's fixed.
Pavel Shilovsky May 26, 2011, 6:06 a.m. UTC | #4
2011/5/25 Jeff Layton <jlayton@redhat.com>:
> On Wed, 25 May 2011 23:10:33 +0400
> Pavel Shilovsky <piastry@etersoft.ru> wrote:
>
>> 2011/5/24 Jeff Layton <jlayton@redhat.com>:
>> > On Thu,  5 May 2011 13:55:14 +0400
>> > Pavel Shilovsky <piastry@etersoft.ru> wrote:
>> >
>> >> Add cifs_match_super to use in sget to share superblock between mounts
>> >> that have the same //server/sharename, credentials and mount options.
>> >> It helps us to improve performance on work with future SMB2.1 leases.
>> >>
>> >> Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
>> >> ---
>> >>  fs/cifs/cifsfs.c   |   18 ++++++++++-
>> >>  fs/cifs/cifsglob.h |    5 +++
>> >>  fs/cifs/connect.c  |   90 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  3 files changed, 112 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> >> index 446b5c2..ba0b7ae 100644
>> >> --- a/fs/cifs/cifsfs.c
>> >> +++ b/fs/cifs/cifsfs.c
>> >> @@ -644,6 +644,7 @@ cifs_do_mount(struct file_system_type *fs_type,
>> >>       struct super_block *sb;
>> >>       struct cifs_sb_info *cifs_sb;
>> >>       struct smb_vol *volume_info;
>> >> +     struct cifs_mnt_data mnt_data;
>> >>       struct dentry *root;
>> >>       char *copied_data = NULL;
>> >>
>> >> @@ -661,13 +662,21 @@ cifs_do_mount(struct file_system_type *fs_type,
>> >>
>> >>       cifs_setup_cifs_sb(volume_info, cifs_sb);
>> >>
>> >> -     sb = sget(fs_type, NULL, set_anon_super, NULL);
>> >> +     mnt_data.vol = volume_info;
>> >> +     mnt_data.cifs_sb = cifs_sb;
>> >> +
>> >> +     sb = sget(fs_type, cifs_match_super, set_anon_super, &mnt_data);
>> >>       if (IS_ERR(sb)) {
>> >>               kfree(cifs_sb);
>> >>               root = ERR_CAST(sb);
>> >>               goto out;
>> >>       }
>> >>
>> >> +     if (sb->s_fs_info) {
>> >> +             cFYI(1, "Use existing superblock");
>> >> +             goto out_shared;
>> >> +     }
>> >> +
>> >>       sb->s_flags = flags;
>> >>
>> >>       /*
>> >> @@ -700,6 +709,13 @@ out:
>> >>       cifs_cleanup_volume_info(&volume_info);
>> >>       return root;
>> >>
>> >> +out_shared:
>> >> +     root = cifs_get_root(volume_info, sb);
>> >> +     if (root)
>> >> +             cFYI(1, "dentry root is: %p", root);
>> >> +     cifs_cleanup_volume_info(&volume_info);
>> >> +     return root;
>> >> +
>> >>  err_out:
>> >>       kfree(cifs_sb);
>> >>       deactivate_locked_super(sb);
>> >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> >> index 7ad7d69..55901b0 100644
>> >> --- a/fs/cifs/cifsglob.h
>> >> +++ b/fs/cifs/cifsglob.h
>> >> @@ -211,6 +211,11 @@ struct smb_vol {
>> >>       struct nls_table *local_nls;
>> >>  };
>> >>
>> >> +struct cifs_mnt_data {
>> >> +     struct cifs_sb_info *cifs_sb;
>> >> +     struct smb_vol *vol;
>> >> +};
>> >> +
>> >>  struct TCP_Server_Info {
>> >>       struct list_head tcp_ses_list;
>> >>       struct list_head smb_ses_list;
>> >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> >> index 1cacfaa..1d39dcf 100644
>> >> --- a/fs/cifs/connect.c
>> >> +++ b/fs/cifs/connect.c
>> >> @@ -2135,6 +2135,96 @@ cifs_put_tlink(struct tcon_link *tlink)
>> >>       return;
>> >>  }
>> >>
>> >> +static inline struct tcon_link *
>> >> +cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb);
>> >> +
>> >> +static const u32 ACTUAL_MOUNT_FLAGS = CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
>> >> +     CIFS_MOUNT_SERVER_INUM | CIFS_MOUNT_DIRECT_IO | CIFS_MOUNT_NO_XATTR |
>> >> +     CIFS_MOUNT_MAP_SPECIAL_CHR | CIFS_MOUNT_UNX_EMUL | CIFS_MOUNT_NO_BRL |
>> >> +     CIFS_MOUNT_CIFS_ACL | CIFS_MOUNT_OVERR_UID | CIFS_MOUNT_OVERR_GID |
>> >> +     CIFS_MOUNT_DYNPERM | CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC |
>> >> +     CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | CIFS_MOUNT_MULTIUSER |
>> >> +     CIFS_MOUNT_STRICT_IO;
>> >> +
>> >> +static int
>> >> +compare_mount_options(struct cifs_sb_info *old, struct cifs_sb_info *new)
>> >> +{
>> >> +     if ((old->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS) !=
>> >> +         (new->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS))
>> >> +             return 0;
>> >> +
>> >> +     if (old->rsize != new->rsize || old->wsize != new->wsize)
>> >> +             return 0;
>> >> +
>> >                ^^^^^^^^^^
>> > The rsize and wsize are negotiated -- you can't know what this is until
>> > you talk to the server. Doesn't this check always fail?
>>
>> If we don't specify wsize and rsize they are set to default values
>> (16384 and 57344) in both mounts, this check succeed.
>>
>
> Ahh then this will likely be broken if/when the wsize negotiation patch
> goes in. I suppose we'll have to see which patch (if any) Steve decides
> to commit first and then try to fix this up afterward.
>
> The brokenness shouldn't be too bad, you just won't be able to actually
> share superblocks until it's fixed.
>
> --
> Jeff Layton <jlayton@redhat.com>
>

Updated patch 4/4 with wsize changes and posted to the list.
diff mbox

Patch

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 446b5c2..ba0b7ae 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -644,6 +644,7 @@  cifs_do_mount(struct file_system_type *fs_type,
 	struct super_block *sb;
 	struct cifs_sb_info *cifs_sb;
 	struct smb_vol *volume_info;
+	struct cifs_mnt_data mnt_data;
 	struct dentry *root;
 	char *copied_data = NULL;
 
@@ -661,13 +662,21 @@  cifs_do_mount(struct file_system_type *fs_type,
 
 	cifs_setup_cifs_sb(volume_info, cifs_sb);
 
-	sb = sget(fs_type, NULL, set_anon_super, NULL);
+	mnt_data.vol = volume_info;
+	mnt_data.cifs_sb = cifs_sb;
+
+	sb = sget(fs_type, cifs_match_super, set_anon_super, &mnt_data);
 	if (IS_ERR(sb)) {
 		kfree(cifs_sb);
 		root = ERR_CAST(sb);
 		goto out;
 	}
 
+	if (sb->s_fs_info) {
+		cFYI(1, "Use existing superblock");
+		goto out_shared;
+	}
+
 	sb->s_flags = flags;
 
 	/*
@@ -700,6 +709,13 @@  out:
 	cifs_cleanup_volume_info(&volume_info);
 	return root;
 
+out_shared:
+	root = cifs_get_root(volume_info, sb);
+	if (root)
+		cFYI(1, "dentry root is: %p", root);
+	cifs_cleanup_volume_info(&volume_info);
+	return root;
+
 err_out:
 	kfree(cifs_sb);
 	deactivate_locked_super(sb);
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 7ad7d69..55901b0 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -211,6 +211,11 @@  struct smb_vol {
 	struct nls_table *local_nls;
 };
 
+struct cifs_mnt_data {
+	struct cifs_sb_info *cifs_sb;
+	struct smb_vol *vol;
+};
+
 struct TCP_Server_Info {
 	struct list_head tcp_ses_list;
 	struct list_head smb_ses_list;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 1cacfaa..1d39dcf 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2135,6 +2135,96 @@  cifs_put_tlink(struct tcon_link *tlink)
 	return;
 }
 
+static inline struct tcon_link *
+cifs_sb_master_tlink(struct cifs_sb_info *cifs_sb);
+
+static const u32 ACTUAL_MOUNT_FLAGS = CIFS_MOUNT_NO_PERM | CIFS_MOUNT_SET_UID |
+	CIFS_MOUNT_SERVER_INUM | CIFS_MOUNT_DIRECT_IO | CIFS_MOUNT_NO_XATTR |
+	CIFS_MOUNT_MAP_SPECIAL_CHR | CIFS_MOUNT_UNX_EMUL | CIFS_MOUNT_NO_BRL |
+	CIFS_MOUNT_CIFS_ACL | CIFS_MOUNT_OVERR_UID | CIFS_MOUNT_OVERR_GID |
+	CIFS_MOUNT_DYNPERM | CIFS_MOUNT_NOPOSIXBRL | CIFS_MOUNT_NOSSYNC |
+	CIFS_MOUNT_FSCACHE | CIFS_MOUNT_MF_SYMLINKS | CIFS_MOUNT_MULTIUSER |
+	CIFS_MOUNT_STRICT_IO;
+
+static int
+compare_mount_options(struct cifs_sb_info *old, struct cifs_sb_info *new)
+{
+	if ((old->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS) !=
+	    (new->mnt_cifs_flags & ACTUAL_MOUNT_FLAGS))
+		return 0;
+
+	if (old->rsize != new->rsize || old->wsize != new->wsize)
+		return 0;
+
+	if (old->mnt_uid != new->mnt_uid || old->mnt_gid != new->mnt_gid)
+		return 0;
+
+	if (old->mnt_file_mode != new->mnt_file_mode ||
+	    old->mnt_dir_mode != new->mnt_dir_mode)
+		return 0;
+
+	if (strcmp(old->local_nls->charset, new->local_nls->charset))
+		return 0;
+
+	if (old->actimeo != new->actimeo)
+		return 0;
+
+	return 1;
+}
+
+int
+cifs_match_super(struct super_block *sb, void *data)
+{
+	struct cifs_mnt_data *mnt_data = (struct cifs_mnt_data *)data;
+	struct smb_vol *volume_info;
+	struct cifs_sb_info *cifs_sb, *new_cifs_sb;
+	struct TCP_Server_Info *tcp_srv;
+	struct cifsSesInfo *ses;
+	struct cifsTconInfo *tcon;
+	struct tcon_link *tlink;
+	struct sockaddr_storage addr;
+	int rc = 0;
+
+	memset(&addr, 0, sizeof(struct sockaddr_storage));
+
+	spin_lock(&cifs_tcp_ses_lock);
+	cifs_sb = CIFS_SB(sb);
+	tlink = cifs_get_tlink(cifs_sb_master_tlink(cifs_sb));
+	if (IS_ERR(tlink)) {
+		spin_unlock(&cifs_tcp_ses_lock);
+		return rc;
+	}
+	tcon = tlink_tcon(tlink);
+	ses = tcon->ses;
+	tcp_srv = ses->server;
+
+	volume_info = mnt_data->vol;
+	new_cifs_sb = mnt_data->cifs_sb;
+
+	if (!volume_info->UNCip || !volume_info->UNC)
+		goto out;
+
+	rc = cifs_fill_sockaddr((struct sockaddr *)&addr,
+				volume_info->UNCip,
+				strlen(volume_info->UNCip),
+				volume_info->port);
+	if (!rc)
+		goto out;
+
+	if (!match_server(tcp_srv, (struct sockaddr *)&addr, volume_info) ||
+	    !match_session(ses, volume_info) ||
+	    !match_tcon(tcon, volume_info->UNC)) {
+		rc = 0;
+		goto out;
+	}
+
+	rc = compare_mount_options(cifs_sb, new_cifs_sb);
+out:
+	cifs_put_tlink(tlink);
+	spin_unlock(&cifs_tcp_ses_lock);
+	return rc;
+}
+
 int
 get_dfs_path(int xid, struct cifsSesInfo *pSesInfo, const char *old_path,
 	     const struct nls_table *nls_codepage, unsigned int *pnum_referrals,