lingering caps outstanding after client shutdown?

lingering caps outstanding after client shutdown?

[Jeff Layton]

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

In order to get the correct semantics for a delegation or oplock, we
need to be able to break delegations on open. Cephfs' open routine is
pretty light with the caps though.

One way to fix that is to request a enough caps to cause a delegation
break if any are outstanding. So, I've been playing with the attached
patch. It may be wrong and not what we want, but it seems like it ought
to work.

This patch however causes the LibCephFS.Fchmod test to hang, waiting on
caps, iff the DoubleChmod test is run before it. If you run just Fchmod
test, it works fine. They both use the same filename and hence the same
inode.

The test hangs at the first Fchmod test ceph_open call, waiting for the
caps. At that point though, there should be no other clients accessing
the mount. Is it possible that outstanding caps granted to the client
from the earlier test could be surviving a ceph_shutdown, such that it
blocks the next client from getting them?

To reproduce I've been running this on a build with the attached patch
vs. a vstart cluster:

    $ ./bin/ceph_test_libcephfs --gtest_filter=LibCephFS.DoubleChmod:LibCephFS.Fchmod

I'll continue to dig into this, but I'm starting to wonder if this is
exposing a bug in the mds. Should I open a tracker bug on this?
-- 
Jeff Layton <jlayton@redhat.com>

[-- Attachment #2: 0001-client-request-caps-on-open.patch --]
[-- Type: text/x-patch, Size: 1612 bytes --]

From 6f2d562f06529631235bea02b821f83a1a3d0e73 Mon Sep 17 00:00:00 2001
From: Jeffrey Layton <jlayton@ceres.poochiereds.net>
Date: Mon, 25 Sep 2017 15:09:48 -0400
Subject: [PATCH] client: request caps on open

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 src/client/Client.cc | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 78517c808a..ffe877fdf8 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8425,11 +8425,22 @@ int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp,
 
   in->get_open_ref(cmode);  // make note of pending open, since it effects _wanted_ caps.
 
-  if ((flags & O_TRUNC) == 0 &&
-      in->caps_issued_mask(want)) {
+  if ((flags & O_TRUNC) == 0 && in->caps_issued_mask(want)) {
     // update wanted?
     check_caps(in, CHECK_CAPS_NODELAY);
   } else {
+    int need = 0, have;
+
+    /* Wait on minimal caps, just to break delegations */
+    if (cmode & CEPH_FILE_MODE_WR)
+      need |= CEPH_CAP_FILE_WR;
+    if (cmode & CEPH_FILE_MODE_RD)
+      need |= CEPH_CAP_FILE_RD;
+
+    result = get_caps(in, need, want, &have, -1);
+    if (result < 0)
+      return result;
+
     MetaRequest *req = new MetaRequest(CEPH_MDS_OP_OPEN);
     filepath path;
     in->make_nosnap_relative_path(path);
@@ -8444,6 +8455,7 @@ int Client::_open(Inode *in, int flags, mode_t mode, Fh **fhp,
     req->head.args.open.old_size = in->size;   // for O_TRUNC
     req->set_inode(in);
     result = make_request(req, perms);
+    put_cap_ref(in, need);
   }
 
   // success?
-- 
2.13.5

Re: lingering caps outstanding after client shutdown?

[Sage Weil]

On Tue, 26 Sep 2017, Jeff Layton wrote:
> In order to get the correct semantics for a delegation or oplock, we
> need to be able to break delegations on open.

Can you explain this part?  How do the delegation/oplock semantics relate 
to open(2)?  I would assume they are related to reads and writes (and data 
consistency).  Unless it's a CIFS thing?

sage

Re: lingering caps outstanding after client shutdown?

[Jeff Layton]

On Tue, 2017-09-26 at 13:31 +0000, Sage Weil wrote:
> On Tue, 26 Sep 2017, Jeff Layton wrote:
> > In order to get the correct semantics for a delegation or oplock,
> we
> > need to be able to break delegations on open.
> 
> Can you explain this part?  How do the delegation/oplock semantics
> relate 
> to open(2)?  I would assume they are related to reads and writes (and
> data 
> consistency).  Unless it's a CIFS thing?


You're correct that we only really need to break delegations on
conflicting access, but there is a problem here:

NFS clients can cache shared/deny mode locks if they hold a delegation.
Once you've granted an open, it's too late to go back and enforce it
once you go to recall the delegation.

Since we don't enforce deny mode locks in cephfs, we could argue that
all of that is the purview of the NFS server and not worry about
it...at least until we toss something like samba into the mix, at which
point things get a bit more complicated...

-- 
Jeff Layton <jlayton@redhat.com>

Re: lingering caps outstanding after client shutdown?

[Jeff Layton]

On Tue, 2017-09-26 at 10:06 -0400, Jeff Layton wrote:
> On Tue, 2017-09-26 at 13:31 +0000, Sage Weil wrote:
> > On Tue, 26 Sep 2017, Jeff Layton wrote:
> > > In order to get the correct semantics for a delegation or oplock,
> > 
> > we
> > > need to be able to break delegations on open.
> > 
> > Can you explain this part?  How do the delegation/oplock semantics
> > relate 
> > to open(2)?  I would assume they are related to reads and writes (and
> > data 
> > consistency).  Unless it's a CIFS thing?
> 
> 
> You're correct that we only really need to break delegations on
> conflicting access, but there is a problem here:
> 
> NFS clients can cache shared/deny mode locks if they hold a delegation.
> Once you've granted an open, it's too late to go back and enforce it
> once you go to recall the delegation.
> 
> Since we don't enforce deny mode locks in cephfs, we could argue that
> all of that is the purview of the NFS server and not worry about
> it...at least until we toss something like samba into the mix, at which
> point things get a bit more complicated...
> 

After some discussion with Sage on IRC, I think I understand what's
going on now.

The issue is that Client::get_caps doesn't actually request any caps.
It just waits on the needed caps, under the assumption that you've
already requested the ones you need via an earlier call.

In this case, the file already existed before the client connected, and
there is no interaction with the file by that client prior to the open.
 There is no reason that the MDS thinks that the client wants any caps
for this file, and so the call hangs waiting on caps that will never
arrive.

I've been able to fix the patch by having it call get_caps after the
open call. With that, the MDS knows that the client is holding the file
open, and so will want Fr and/or Fw caps for that. That's sufficient to
satisfy the right delegation break semantic.

The upshot here is that this isn't a bug, after all...

Cheers,
-- 
Jeff Layton <jlayton@poochiereds.net>