Received-SPF: pass (google.com: domain of linux-nfs+bounces-2230-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 7 Mar 2024 16:29:43 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: neilb@suse.de
Cc: linux-nfs@vger.kernel.org
Subject: [bug report] nfsd: perform all find_openstateowner_str calls in the
 one place.
Message-ID: <44e5a5f5-f8df-45bd-be1f-7d67cfb011de@moroto.mountain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hello NeilBrown,

Commit 11c3d0a15bbc ("nfsd: perform all find_openstateowner_str calls
in the one place.") from Mar 5, 2024 (linux-next), leads to the
following Smatch static checker warning:

	fs/nfsd/nfs4state.c:1674 release_openowner()
	warn: sleeping in atomic context

fs/nfsd/nfs4state.c
    1657 static void release_openowner(struct nfs4_openowner *oo)
    1658 {
    1659         struct nfs4_ol_stateid *stp;
    1660         struct nfs4_client *clp = oo->oo_owner.so_client;
    1661         struct list_head reaplist;
    1662 
    1663         INIT_LIST_HEAD(&reaplist);
    1664 
    1665         spin_lock(&clp->cl_lock);
    1666         unhash_openowner_locked(oo);
    1667         while (!list_empty(&oo->oo_owner.so_stateids)) {
    1668                 stp = list_first_entry(&oo->oo_owner.so_stateids,
    1669                                 struct nfs4_ol_stateid, st_perstateowner);
    1670                 if (unhash_open_stateid(stp, &reaplist))
    1671                         put_ol_stateid_locked(stp, &reaplist);
    1672         }
    1673         spin_unlock(&clp->cl_lock);
--> 1674         free_ol_stateid_reaplist(&reaplist);
                 ^^^^^^^^^^^^^^^^^^^^^^^^
This is a might sleep function.

    1675         release_last_closed_stateid(oo);
    1676         nfs4_put_stateowner(&oo->oo_owner);
    1677 }

The caller is find_or_alloc_open_stateowner()

fs/nfsd/nfs4state.c
  4863  find_or_alloc_open_stateowner(unsigned int strhashval, struct nfsd4_open *open,
  4864                                struct nfsd4_compound_state *cstate)
  4865  {
  4866          struct nfs4_client *clp = cstate->clp;
  4867          struct nfs4_openowner *oo, *new = NULL;
  4868  
  4869          while (1) {
  4870                  spin_lock(&clp->cl_lock);
                       ^^^^^^^^^^^^^^^^^^^^^^^^^
Huh...  This looks like the same lock that we take in
release_openowner().  Why do I not see a static checker warning for
double lock?


  4871                  oo = find_openstateowner_str(strhashval, open, clp);
  4872                  if (oo && !(oo->oo_flags & NFS4_OO_CONFIRMED)) {
  4873                          /* Replace unconfirmed owners without checking for replay. */
  4874                          release_openowner(oo);
                                ^^^^^^^^^^^^^^^^^^^^^^
Here

  4875                          oo = NULL;
  4876                  }
  4877                  if (oo) {
  4878                          spin_unlock(&clp->cl_lock);
  4879                          if (new)
  4880                                  nfs4_free_stateowner(&new->oo_owner);
  4881                          return oo;
  4882                  }
  4883                  if (new) {
  4884                          hash_openowner(new, clp, strhashval);
  4885                          spin_unlock(&clp->cl_lock);
  4886                          return new;
  4887                  }
  4888                  spin_unlock(&clp->cl_lock);


regards,
dan carpenter