public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Neil Brown <neilb@suse.de>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 00/12] Some improvements to request deferral and related code
Date: Fri, 7 Aug 2009 14:13:50 +1000	[thread overview]
Message-ID: <19067.43518.105153.247173@notabene.brown> (raw)
In-Reply-To: message from J. Bruce Fields on Tuesday August 4

On Tuesday August 4, bfields@fieldses.org wrote:
> On Tue, Aug 04, 2009 at 03:22:38PM +1000, NeilBrown wrote:
> >  This series fixes a few little bugs and tidies up some code but does
> >  two main important things.
> > 
> >  1/ 'allow thread to block....' will wait a little while if there is a
> >  cache miss.  If an answer is available in that time, it continues on
> >  it's merry way.  If no answer arrives, the old deferral approach is
> >  used.  It waits 5 seconds if there are spare nfsd threads, and 1
> >  second if there all threads are busy.  I have almost nothing with
> >  which to justify these numbers.
> 
> I think the v4 server at least should return NFS4ERR_DELAY in this case
> instead of doing the internal replay.  That avoids possible problems
> with non-idempotent compound ops.

If the request has been handed to nfsd, then yes I agree.  We probably
want some way for nfsd to mark the request as "don't replay" so that
an error will propagate out.  Currently we map that error to EJUKEBOX
for v3 or v4, but you are right, we want ERR_DELAY for v4.

If the request is still in the RPC code (trying to identify the
origin or to decode the crypto) then we cannot return ERR_DELAY, but
as none of the request will have been processed yet, there is no room
for a problem with non-idempotent ops.

It has occurred to me that we could throw away the current request
deferral completely:  if we don't feel comfortable delaying the thread
for as long as it takes, we just return an error or drop the request
(closing any connection).
I'm not sure I'd be comfortable doing that if there were only a few
(8?) threads though.
Maybe if we got dynamic nfsd threads so that new ones could be created
on demand I would feel quite happy to discard the deferral stuff and
just use a delay.

> 
> >From the protocol point of view I don't know if there's any rule of
> thumb about when it'd be best to return DELAY.  Perhaps it's best to
> avoid it whenever possible, but when the delay is on the order of
> seconds it sounds reasonable to me.

Of course you don't know how long the delay will be until it happens:-)

But I agree.  Delay internally if possible, but as soon as that seems
to be awkward (e.g. run out of threads), return DELAY

Thanks,
NeilBrown

  reply	other threads:[~2009-08-07  4:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-04  5:22 [PATCH 00/12] Some improvements to request deferral and related code NeilBrown
     [not found] ` <20090804051145.15929.11356.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04  5:22   ` [PATCH 04/12] sunrpc/cache: recheck cache validity after cache_defer_req NeilBrown
     [not found]     ` <20090804052238.15929.56800.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 20:42       ` J. Bruce Fields
2009-08-06  4:57         ` Neil Brown
     [not found]           ` <19066.25248.283061.383233-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-25 21:50             ` J. Bruce Fields
2009-08-26  0:42               ` Neil Brown
2009-08-04  5:22   ` [PATCH 02/12] sunrpc/cache: make sure deferred requests eventually get revisited NeilBrown
     [not found]     ` <20090804052238.15929.74402.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 15:02       ` J. Bruce Fields
2009-08-04  5:22   ` [PATCH 01/12] sunrpc/cache: rename queue_loose to cache_dequeue NeilBrown
     [not found]     ` <20090804052238.15929.91015.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 14:05       ` J. Bruce Fields
2009-08-04  5:22   ` [PATCH 03/12] sunrpc/cache: simplify cache_fresh_locked and cache_fresh_unlocked NeilBrown
     [not found]     ` <20090804052238.15929.17142.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 15:45       ` J. Bruce Fields
2009-08-04  5:22   ` [PATCH 05/12] sunrpc/cache: use list_del_init for the list_head entries in cache_deferred_req NeilBrown
2009-08-04  5:22   ` [PATCH 11/12] sunrpc/cache: change deferred-request hash table to use hlist NeilBrown
2009-08-04  5:22   ` [PATCH 10/12] sunrpc: fix memory leak in unix_gid cache NeilBrown
     [not found]     ` <20090804052239.15929.71459.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 20:55       ` J. Bruce Fields
2009-08-04  5:22   ` [PATCH 08/12] sunrpc/cache: retry cache lookups that return -ETIMEDOUT NeilBrown
2009-08-04  5:22   ` [PATCH 09/12] nfsd/idmap: drop special request deferal in favour of improved default NeilBrown
2009-08-04  5:22   ` [PATCH 06/12] sunrpc/cache: avoid variable over-loading in cache_defer_req NeilBrown
     [not found]     ` <20090804052239.15929.87201.stgit-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-04 20:47       ` J. Bruce Fields
2009-08-06  4:35         ` Neil Brown
2009-08-04  5:22   ` [PATCH 07/12] sunrpc/cache: allow thread to block while waiting for cache update NeilBrown
2009-08-04  5:22   ` [PATCH 12/12] sunrpc: close connection when a request is irretrievably lost NeilBrown
2009-08-04 14:04   ` [PATCH 00/12] Some improvements to request deferral and related code J. Bruce Fields
2009-08-07  4:13     ` Neil Brown [this message]
     [not found]       ` <19067.43518.105153.247173-wvvUuzkyo1EYVZTmpyfIwg@public.gmane.org>
2009-08-10 15:05         ` J. Bruce Fields

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=19067.43518.105153.247173@notabene.brown \
    --to=neilb@suse.de \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox