All of lore.kernel.org
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.de>
To: Steve Dickson <SteveD@redhat.com>
Cc: Jeff Layton <jlayton@redhat.com>,
	trond.myklebust@netapp.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 0/2] sunrpc: more reliable detection of running gssd
Date: Wed, 13 Nov 2013 09:15:37 +1100	[thread overview]
Message-ID: <20131113091537.059162de@notabene.brown> (raw)
In-Reply-To: <52824784.4080901@RedHat.com>

[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]

On Tue, 12 Nov 2013 10:21:40 -0500 Steve Dickson <SteveD@redhat.com> wrote:

> On 12/11/13 08:00, Jeff Layton wrote:
> > We've gotten a lot of complaints recently about the 15s delay when
> > doing a sec=sys mount without gssd running.
> > 
> > A large part of the problem is that the kernel isn't able to reliably
> > detect when rpc.gssd is running. What we currently have is a
> > gssd_running flag that is initially set to 1. When an upcall times out,
> > that gets set to 0, and subsequent upcalls get a much shorter timeout
> > (1/4s instead of 15s). It's reset back to '1' when a pipe is reopened.
> > 
> > The approach of using a flag like this is pretty inadequate. First, it
> > doesn't eliminate the long delay on the initial upcall attempt. Also,
> > if gssd spontaneously dies, then the flag will still be set to 1 until
> > the next upcall attempt times out. Finally, it currently requires that
> > the pipe be reopened in order to reset the flag back to true.
> > 
> > This patchset replaces that flag with a more reliable mechanism for
> > detecting when gssd is running. When rpc_pipefs is mounted, it creates a
> > new "dummy" pipe that gssd will naturally find and hold open. We'll
> > never send an upcall down this pipe, and writing to it always fails.
> > But, since we can detect when something is holding it open, we can use
> > that to determine whether gssd is running.
> > 
> > The current patch just uses this mechanism to replace the gssd_running
> > flag with this new mechanism. This shortens the long delay when mounting
> > without gssd running, but does not silence these warnings:
> > 
> >     RPC: AUTH_GSS upcall timed out.
> >     Please check user daemon is running.
> > 
> > I'm willing to add a patch to do that, but I'm a little unclear on the
> > best way to do so. Those messages are generated by the auth_gss code. We
> > probably do want to print them if someone mounted with sec=krb5, but
> > suppress them when mounting with sec=sys.
> > 
> > Do we need to somehow pass down that intent to auth_gss? Another idea
> > would be to call gssd_running() from the nfs mount code and use that to
> > determine whether to try and use krb5 at all...
> > 
> > Discuss!
> I've just verified that a mount, with these patches, takes about 
> 1.2 seconds when rpc.gssd is not running.... With rpc.gssd it 
> take about .2 seconds.
> 
> Tested-by: Steve Dickson <steved@redhat.com>
>

Still sounds like about one second too long.

In that patch I see:

 	timeout = 15 * HZ;
-	if (!sn->gssd_running)
+	if (!gssd_running(sn))
 		timeout = HZ >> 2;

Given that "!gssd_running(sn)" is now certain knowledge rather than a hint,
can't we just skip the upcall and any timeout?
i.e.
 	timeout = 15 * HZ;
-	if (!sn->gssd_running)
+	if (!gssd_running(sn))
- 		timeout = HZ >> 2;
+		return -EACCES;

??

NeilBrown


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

  reply	other threads:[~2013-11-12 22:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-12 13:00 [PATCH 0/2] sunrpc: more reliable detection of running gssd Jeff Layton
2013-11-12 13:00 ` [PATCH 1/2] sunrpc: create a new dummy pipe for gssd to hold open Jeff Layton
2013-11-12 17:36   ` Myklebust, Trond
2013-11-12 17:44     ` Jeff Layton
2013-11-12 17:56       ` Myklebust, Trond
2013-11-12 13:00 ` [PATCH 2/2] sunrpc: replace sunrpc_net->gssd_running flag with a better mechanism Jeff Layton
2013-11-12 15:21 ` [PATCH 0/2] sunrpc: more reliable detection of running gssd Steve Dickson
2013-11-12 22:15   ` NeilBrown [this message]
2013-11-12 22:37     ` Jeff Layton
2013-11-12 16:02 ` Chuck Lever
2013-11-12 16:08   ` Jeff Layton
2013-11-12 16:15     ` Chuck Lever
2013-11-12 16:56       ` Myklebust, Trond
2013-11-12 17:12         ` Chuck Lever
2013-11-12 17:13         ` Chuck Lever

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=20131113091537.059162de@notabene.brown \
    --to=neilb@suse.de \
    --cc=SteveD@redhat.com \
    --cc=jlayton@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trond.myklebust@netapp.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.