Date:   Mon, 8 Apr 2019 12:41:09 -0400
From:   "J. Bruce Fields" <bfields@redhat.com>
To:     Trond Myklebust <trondmy@hammerspace.com>
Cc:     "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v2] nfsd: Don't release the callback slot unless it was
 actually held
Message-ID: <20190408164109.GA18799@parsley.fieldses.org>
References: <20190405155437.5545-1-trond.myklebust@hammerspace.com>
 <4bf043aa23b4230bf41fbcd5611b3baff4977f1d.camel@hammerspace.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4bf043aa23b4230bf41fbcd5611b3baff4977f1d.camel@hammerspace.com>
User-Agent: Mutt/1.11.3 (2019-02-01)
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 08, 2019 at 03:20:48PM +0000, Trond Myklebust wrote:
> On Fri, 2019-04-05 at 08:54 -0700, Trond Myklebust wrote:
> > If there are multiple callbacks queued, waiting for the callback
> > slot when the callback gets shut down, then they all currently
> > end up acting as if they hold the slot, and call
> > nfsd4_cb_sequence_done() resulting in interesting side-effects.
> > 
> > In addition, the 'retry_nowait' path in nfsd4_cb_sequence_done()
> > causes a loop back to nfsd4_cb_prepare() without first freeing the
> > slot, which causes a deadlock when nfsd41_cb_get_slot() gets called
> > a second time.
> > 
> > This patch therefore adds a boolean to track whether or not the
> > callback did pick up the slot, so that it can do the right thing
> > in these 2 cases.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> > ---
> > v2: try to restart the callback if we hit nfsd4_cb_sequence_done()
> >     without a slot.
> > 
> > 
> 
> Hi Bruce,
> 
> Should this patch perhaps be considered for stable? The callback slot
> leak is permanent (or at least for the lifetime of the nfs4_client).

Makes sense to me; I'll queue it up for 5.1 and stable.

What were the original symptoms that prompted this?

--b.