All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/3] nfsd: fix callback restarts
Date: Thu, 30 Apr 2015 16:35:56 -0400	[thread overview]
Message-ID: <20150430203556.GA9509@fieldses.org> (raw)
In-Reply-To: <1430387365-24348-3-git-send-email-hch@lst.de>

Looks good to me, just a changelog nit:

On Thu, Apr 30, 2015 at 11:49:24AM +0200, Christoph Hellwig wrote:
> Checking the rpc_client pointer is not a reliable way to detect a backchannel
> connetion failure, as the likelyhood of reusing the same slab object is
> very high.

I don't think that's true, if it's this comparison you're talking about:

> @@ -907,16 +900,21 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
>  			clp->cl_cb_session->se_cb_seq_nr);
>  	}
>  
> -	if (clp->cl_cb_client != task->tk_client) {
> -		/* We're shutting down or changing cl_cb_client; leave
> -		 * it to nfsd4_process_cb_update to restart the call if
> -		 * necessary. */

This is an rpc callback, so tk_client better still be allocated.
cl_cb_client too, since as far as I can tell that's never changed
without shutting down the rpc client first (which will wait for all
tasks to exit).

So I agree that this is wrong, but I think the reason it's actually
wrong is that the condition is just never true....

--b.

  reply	other threads:[~2015-04-30 20:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-30  9:49 nfsd: callback fixes Christoph Hellwig
2015-04-30  9:49 ` [PATCH 1/3] nfsd: split transport vs operation errors for callbacks Christoph Hellwig
2015-04-30 14:24   ` J. Bruce Fields
2015-04-30 14:38     ` Christoph Hellwig
2015-04-30  9:49 ` [PATCH 2/3] nfsd: fix callback restarts Christoph Hellwig
2015-04-30 20:35   ` J. Bruce Fields [this message]
2015-05-01  8:44     ` Christoph Hellwig
2015-04-30  9:49 ` [PATCH 3/3] nfsd: skip CB_NULL probes for 4.1 or later Christoph Hellwig

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=20150430203556.GA9509@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=hch@lst.de \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@primarydata.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.