public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Trond Myklebust <trond.myklebust@hammerspace.com>
Cc: linux-nfs@vger.kernel.org
Subject: [bug report] NFSv4: Deal more correctly with duplicate delegations
Date: Mon, 22 Mar 2021 16:40:15 +0300	[thread overview]
Message-ID: <YFieP4B3XmHsUn/a@mwanda> (raw)

Hello Trond Myklebust,

The patch 57bfa89171e5: "NFSv4: Deal more correctly with duplicate
delegations" from Jan 25, 2008, leads to the following static checker
warning:

	fs/nfs/delegation.c:482 nfs_inode_set_delegation()
	warn: missing error code here? 'nfs_detach_delegation_locked()' failed. 'status' = '0'

fs/nfs/delegation.c
   421  int nfs_inode_set_delegation(struct inode *inode, const struct cred *cred,
   422                                    fmode_t type,
   423                                    const nfs4_stateid *stateid,
   424                                    unsigned long pagemod_limit)
   425  {
   426          struct nfs_server *server = NFS_SERVER(inode);
   427          struct nfs_client *clp = server->nfs_client;
   428          struct nfs_inode *nfsi = NFS_I(inode);
   429          struct nfs_delegation *delegation, *old_delegation;
   430          struct nfs_delegation *freeme = NULL;
   431          int status = 0;
                    ^^^^^^^^^^
"status" is always success.

   432  
   433          delegation = kmalloc(sizeof(*delegation), GFP_NOFS);
   434          if (delegation == NULL)
   435                  return -ENOMEM;
   436          nfs4_stateid_copy(&delegation->stateid, stateid);
   437          refcount_set(&delegation->refcount, 1);
   438          delegation->type = type;
   439          delegation->pagemod_limit = pagemod_limit;
   440          delegation->change_attr = inode_peek_iversion_raw(inode);
   441          delegation->cred = get_cred(cred);
   442          delegation->inode = inode;
   443          delegation->flags = 1<<NFS_DELEGATION_REFERENCED;
   444          spin_lock_init(&delegation->lock);
   445  
   446          spin_lock(&clp->cl_lock);
   447          old_delegation = rcu_dereference_protected(nfsi->delegation,
   448                                          lockdep_is_held(&clp->cl_lock));
   449          if (old_delegation == NULL)
   450                  goto add_new;
   451          /* Is this an update of the existing delegation? */
   452          if (nfs4_stateid_match_other(&old_delegation->stateid,
   453                                  &delegation->stateid)) {
   454                  spin_lock(&old_delegation->lock);
   455                  nfs_update_inplace_delegation(old_delegation,
   456                                  delegation);
   457                  spin_unlock(&old_delegation->lock);
   458                  goto out;

I think these used to return -EIO back in the day.

   459          }
   460          if (!test_bit(NFS_DELEGATION_REVOKED, &old_delegation->flags)) {
   461                  /*
   462                   * Deal with broken servers that hand out two
   463                   * delegations for the same file.
   464                   * Allow for upgrades to a WRITE delegation, but
   465                   * nothing else.
   466                   */
   467                  dfprintk(FILE, "%s: server %s handed out "
   468                                  "a duplicate delegation!\n",
   469                                  __func__, clp->cl_hostname);
   470                  if (delegation->type == old_delegation->type ||
   471                      !(delegation->type & FMODE_WRITE)) {
   472                          freeme = delegation;
   473                          delegation = NULL;
   474                          goto out;
   475                  }
   476                  if (test_and_set_bit(NFS_DELEGATION_RETURNING,
   477                                          &old_delegation->flags))
   478                          goto out;
   479          }
   480          freeme = nfs_detach_delegation_locked(nfsi, old_delegation, clp);
   481          if (freeme == NULL)
   482                  goto out;

status isn't set

   483  add_new:
   484          list_add_tail_rcu(&delegation->super_list, &server->delegations);
   485          rcu_assign_pointer(nfsi->delegation, delegation);
   486          delegation = NULL;
   487  
   488          atomic_long_inc(&nfs_active_delegations);
   489  
   490          trace_nfs4_set_delegation(inode, type);
   491  
   492          spin_lock(&inode->i_lock);
   493          if (NFS_I(inode)->cache_validity & (NFS_INO_INVALID_ATTR|NFS_INO_INVALID_ATIME))
   494                  NFS_I(inode)->cache_validity |= NFS_INO_REVAL_FORCED;
   495          spin_unlock(&inode->i_lock);
   496  out:
   497          spin_unlock(&clp->cl_lock);
   498          if (delegation != NULL)
   499                  __nfs_free_delegation(delegation);
   500          if (freeme != NULL) {
   501                  nfs_do_return_delegation(inode, freeme, 0);
   502                  nfs_free_delegation(freeme);
   503          }
   504          return status;
   505  }

regards,
dan carpenter

                 reply	other threads:[~2021-03-22 13:41 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YFieP4B3XmHsUn/a@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox