[bug report] cifs: avoid use of global locks for high contention data

[bug report] cifs: avoid use of global locks for high contention data

[Dan Carpenter]

Hello Shyam Prasad N,

The patch fe67bd563ec2: "cifs: avoid use of global locks for high
contention data" from Jul 27, 2022, leads to the following Smatch
static checker warning:

fs/cifs/connect.c:4641 cifs_tree_connect()
warn: inconsistent returns '&tcon->tc_lock'.
  Locked on  : 4587
  Unlocked on: 4641

fs/cifs/connect.c
  4570  int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const struct nls_table *nlsc)
  4571  {
  4572          int rc;
  4573          struct TCP_Server_Info *server = tcon->ses->server;
  4574          const struct smb_version_operations *ops = server->ops;
  4575          struct super_block *sb = NULL;
  4576          struct cifs_sb_info *cifs_sb;
  4577          struct dfs_cache_tgt_list tl = DFS_CACHE_TGT_LIST_INIT(tl);
  4578          char *tree;
  4579          struct dfs_info3_param ref = {0};
  4580  
  4581          /* only send once per connect */
  4582          spin_lock(&tcon->tc_lock);
                           ^^^^^^^^^^^^^

  4583          if (tcon->ses->ses_status != SES_GOOD ||
  4584              (tcon->status != TID_NEW &&
  4585              tcon->status != TID_NEED_TCON)) {
  4586                  spin_unlock(&tcon->ses->ses_lock);
                                     ^^^^^^^^^^^^^^^^^^^
Originally this used to take a lock and then unlock it again but now it
unlocks a different lock.

  4587                  return 0;
  4588          }
  4589          tcon->status = TID_IN_TCON;
  4590          spin_unlock(&tcon->tc_lock);
  4591  

regards,
dan carpenter

Re: [bug report] cifs: avoid use of global locks for high contention data

[Steve French]

this was already fixed by:

commit aea02fc40a7fa6ac2c16e3c3a6f1d0fd7e6faaba
Author: Yang Yingliang <yangyingliang@huawei.com>
Date:   Fri Jul 29 15:49:35 2022 +0800

    cifs: fix wrong unlock before return from cifs_tree_connect()

On Mon, Aug 1, 2022 at 8:14 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Shyam Prasad N,
>
> The patch fe67bd563ec2: "cifs: avoid use of global locks for high
> contention data" from Jul 27, 2022, leads to the following Smatch
> static checker warning:
>
> fs/cifs/connect.c:4641 cifs_tree_connect()
> warn: inconsistent returns '&tcon->tc_lock'.
>   Locked on  : 4587
>   Unlocked on: 4641
>
> fs/cifs/connect.c
>   4570  int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const struct nls_table *nlsc)
>   4571  {
>   4572          int rc;
>   4573          struct TCP_Server_Info *server = tcon->ses->server;
>   4574          const struct smb_version_operations *ops = server->ops;
>   4575          struct super_block *sb = NULL;
>   4576          struct cifs_sb_info *cifs_sb;
>   4577          struct dfs_cache_tgt_list tl = DFS_CACHE_TGT_LIST_INIT(tl);
>   4578          char *tree;
>   4579          struct dfs_info3_param ref = {0};
>   4580
>   4581          /* only send once per connect */
>   4582          spin_lock(&tcon->tc_lock);
>                            ^^^^^^^^^^^^^
>
>   4583          if (tcon->ses->ses_status != SES_GOOD ||
>   4584              (tcon->status != TID_NEW &&
>   4585              tcon->status != TID_NEED_TCON)) {
>   4586                  spin_unlock(&tcon->ses->ses_lock);
>                                      ^^^^^^^^^^^^^^^^^^^
> Originally this used to take a lock and then unlock it again but now it
> unlocks a different lock.
>
>   4587                  return 0;
>   4588          }
>   4589          tcon->status = TID_IN_TCON;
>   4590          spin_unlock(&tcon->tc_lock);
>   4591
>
> regards,
> dan carpenter



-- 
Thanks,

Steve