All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@poochiereds.net>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/4] fs: allow userland tasks to use delayed_fput infrastructure
Date: Mon, 14 Sep 2015 11:21:20 -0400	[thread overview]
Message-ID: <20150914112120.7d70afbe@tlielax.poochiereds.net> (raw)
In-Reply-To: <20150914144837.GA4332@fieldses.org>

On Mon, 14 Sep 2015 10:48:37 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Sep 14, 2015 at 09:45:51AM -0400, Jeff Layton wrote:
> > I'm breaking this piece out of the open file cache work for nfsd to see
> > if we can get this piece settled before I re-post the whole set. If this
> > looks like a reasonable approach we can sort out how it should be merged
> > (either by you directly, or via Bruce's tree with the rest of the open
> > file cache patches).
> > 
> > For those just joining in, some background:
> > 
> > We want to add an open file cache for nfsd to reduce the open/close
> > overhead on READ/WRITE RPCs, and so we can eliminate the raparm cache.
> > The basic idea is to keep a cache of open files, and close them down on
> > certain sorts of activity -- primarily, after an unlink that takes the
> > link count to 0, or before setting a lease.
> > 
> > The setlease part is problematic though. The plan is to have a notifier
> > callback into nfsd from vfs_setlease that will tell nfsd to close any
> > open files that are associated with the inode so we don't block lease
> > attempts solely due to cached but otherwise idle nfsd files. That means
> > that we need to be able to close out the files and ensure that the final
> > __fput runs before we try to set a lease.
> 
> I think I probably asked something similar before, but just to be sure I
> understand....  Do leases really need to be 100% reliable, or can we get
> away with saying "sorry, I don't feel like granting one right now".  An
> entry in the filehandle cache suggests we're likely to recall the thing
> soon anyway.  We use that option to get out of corner cases in the
> delegation case, but I don't know if it makes sense for oplocks.
> 


They don't need to be 100% reliable, but with the current design nfsd
will hold files open in the cache indefinitely, until one of the
following events occurs:

1) the exports cache is flushed (which is always done after unexporting)

2) an unlink event occurs that drops the i_nlink count to zero

3) userland attempts to set a lease

4) the shrinker kicks in

5) nfsd is shut down

So you could easily have a situation where a NFSv3 client does some
WRITE activity, and then an hour later samba comes along and asks for a
lease. I don't think we'd want to block the lease in that situation as
there's no reason to believe that we'd end up recalling it anytime
soon. The NFS client may be long gone at that point.

We could implement some heuristic that proactively closes out open
files that are idle for a certain amount of time. My first pass did
just that actually, but Christoph didn't much care for it, and I think
he was right. That's not as good a design as just keeping them open
until there's a real reason to close them.

-- 
Jeff Layton <jlayton@poochiereds.net>

WARNING: multiple messages have this Message-ID (diff)
From: Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>
To: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 0/4] fs: allow userland tasks to use delayed_fput infrastructure
Date: Mon, 14 Sep 2015 11:21:20 -0400	[thread overview]
Message-ID: <20150914112120.7d70afbe@tlielax.poochiereds.net> (raw)
In-Reply-To: <20150914144837.GA4332-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

On Mon, 14 Sep 2015 10:48:37 -0400
"J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> wrote:

> On Mon, Sep 14, 2015 at 09:45:51AM -0400, Jeff Layton wrote:
> > I'm breaking this piece out of the open file cache work for nfsd to see
> > if we can get this piece settled before I re-post the whole set. If this
> > looks like a reasonable approach we can sort out how it should be merged
> > (either by you directly, or via Bruce's tree with the rest of the open
> > file cache patches).
> > 
> > For those just joining in, some background:
> > 
> > We want to add an open file cache for nfsd to reduce the open/close
> > overhead on READ/WRITE RPCs, and so we can eliminate the raparm cache.
> > The basic idea is to keep a cache of open files, and close them down on
> > certain sorts of activity -- primarily, after an unlink that takes the
> > link count to 0, or before setting a lease.
> > 
> > The setlease part is problematic though. The plan is to have a notifier
> > callback into nfsd from vfs_setlease that will tell nfsd to close any
> > open files that are associated with the inode so we don't block lease
> > attempts solely due to cached but otherwise idle nfsd files. That means
> > that we need to be able to close out the files and ensure that the final
> > __fput runs before we try to set a lease.
> 
> I think I probably asked something similar before, but just to be sure I
> understand....  Do leases really need to be 100% reliable, or can we get
> away with saying "sorry, I don't feel like granting one right now".  An
> entry in the filehandle cache suggests we're likely to recall the thing
> soon anyway.  We use that option to get out of corner cases in the
> delegation case, but I don't know if it makes sense for oplocks.
> 


They don't need to be 100% reliable, but with the current design nfsd
will hold files open in the cache indefinitely, until one of the
following events occurs:

1) the exports cache is flushed (which is always done after unexporting)

2) an unlink event occurs that drops the i_nlink count to zero

3) userland attempts to set a lease

4) the shrinker kicks in

5) nfsd is shut down

So you could easily have a situation where a NFSv3 client does some
WRITE activity, and then an hour later samba comes along and asks for a
lease. I don't think we'd want to block the lease in that situation as
there's no reason to believe that we'd end up recalling it anytime
soon. The NFS client may be long gone at that point.

We could implement some heuristic that proactively closes out open
files that are idle for a certain amount of time. My first pass did
just that actually, but Christoph didn't much care for it, and I think
he was right. That's not as good a design as just keeping them open
until there's a real reason to close them.

-- 
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-09-14 15:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-14 13:45 [PATCH 0/4] fs: allow userland tasks to use delayed_fput infrastructure Jeff Layton
2015-09-14 13:45 ` Jeff Layton
2015-09-14 13:45 ` [PATCH 1/4] fs: have flush_delayed_fput flush the workqueue job Jeff Layton
2015-09-14 13:45 ` [PATCH 2/4] fs: add a kerneldoc header to fput Jeff Layton
2015-09-14 13:45 ` [PATCH 3/4] fs: add fput_queue Jeff Layton
2015-09-14 13:45   ` Jeff Layton
2015-09-14 14:15   ` Al Viro
2015-09-14 14:19     ` Jeff Layton
2015-09-14 16:39       ` Al Viro
2015-09-14 17:30         ` Jeff Layton
2015-09-14 13:45 ` [PATCH 4/4] fs: export flush_delayed_fput Jeff Layton
2015-09-14 14:48 ` [PATCH 0/4] fs: allow userland tasks to use delayed_fput infrastructure J. Bruce Fields
2015-09-14 14:48   ` J. Bruce Fields
2015-09-14 15:21   ` Jeff Layton [this message]
2015-09-14 15:21     ` Jeff Layton

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=20150914112120.7d70afbe@tlielax.poochiereds.net \
    --to=jlayton@poochiereds.net \
    --cc=bfields@fieldses.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /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.