[bug report] NFSv4: Deal more correctly with duplicate delegations

[bug report] NFSv4: Deal more correctly with duplicate delegations

[Dan Carpenter]

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