deleg: return delegation before conflicting operations

new function nfs41_delegation_return() for synchronous delegation return.  uses a condition variable to wait if another thread is already returning the delegation
if nfs41_delegate_open() would conflict with a delegation, return it before sending the OPEN
return the delegation before sending LINK, RENAME, REMOVE, and SETATTR

all of this functionality is dependent on the preprocessor define DELEGATION_RETURN_ON_CONFLICT (on by default).  if not defined, nfs41_delegation_return() is a noop

Signed-off-by: Casey Bodley <cbodley@citi.umich.edu>

daemon/acl.c

@@ -24,8 +24,8 @@
 #include <strsafe.h>
 #include <sddl.h>
 
-#include "nfs41.h"
 #include "nfs41_ops.h"
+#include "delegation.h"
 #include "daemon_debug.h"
 #include "util.h"
 #include "upcall.h"
@@ -785,6 +785,10 @@ static int handle_setacl(nfs41_upcall *upcall)
         }
     }
 
+    /* break read delegations before SETATTR */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_READ, FALSE);
+
     nfs41_open_stateid_arg(state, &stateid);
     status = nfs41_setattr(state->session, &state->file, &stateid, &info);
     if (status) {

daemon/delegation.c

@@ -50,7 +50,9 @@ static int delegation_create(
     path_fh_init(&state->file, &state->path);
     fh_copy(&state->file.fh, &file->fh);
     list_init(&state->client_entry);
+    state->status = DELEGATION_GRANTED;
     InitializeSRWLock(&state->lock);
+    InitializeConditionVariable(&state->cond);
     state->ref_count = 1;
     *deleg_out = state;
 out:
@@ -148,6 +150,12 @@ static void delegation_return(
     }
     LeaveCriticalSection(&client->state.lock);
 
+    /* signal threads waiting on delegreturn */
+    AcquireSRWLockExclusive(&deleg->lock);
+    deleg->status = DELEGATION_RETURNED;
+    WakeAllConditionVariable(&deleg->cond);
+    ReleaseSRWLockExclusive(&deleg->lock);
+
     /* release the client's reference */
     nfs41_delegation_deref(deleg);
 }
@@ -267,10 +275,28 @@ int nfs41_delegate_open(
     if (status)
         goto out;
 
-    if (!delegation_compatible(deleg->state.type, create, access, deny)) {
+    AcquireSRWLockExclusive(&deleg->lock);
+    if (deleg->status != DELEGATION_GRANTED) {
+        /* the delegation is being returned, wait for it to finish */
+        while (deleg->status != DELEGATION_RETURNED)
+            SleepConditionVariableSRW(&deleg->cond, &deleg->lock, INFINITE, 0);
         status = NFS4ERR_BADHANDLE;
-        goto out_deleg;
     }
+    else if (!delegation_compatible(deleg->state.type, create, access, deny)) {
+#ifdef DELEGATION_RETURN_ON_CONFLICT
+        /* this open will conflict, start the delegation return */
+        deleg->status = DELEGATION_RETURNING;
+        status = NFS4ERR_DELEG_REVOKED;
+#else
+        status = NFS4ERR_BADHANDLE;
+#endif
+    }
+    ReleaseSRWLockExclusive(&deleg->lock);
+
+    if (status == NFS4ERR_DELEG_REVOKED)
+        goto out_return;
+    if (status)
+        goto out_deleg;
 
     /* TODO: check access against deleg->state.permissions or send ACCESS */
 
@@ -279,6 +305,9 @@ int nfs41_delegate_open(
 out:
     return status;
 
+out_return:
+    delegation_return(client, deleg, create == OPEN4_CREATE);
+
 out_deleg:
     nfs41_delegation_deref(deleg);
     goto out;
@@ -344,6 +373,51 @@ out_unlock:
 }
 
 
+/* synchronous delegation return */
+#ifdef DELEGATION_RETURN_ON_CONFLICT
+int nfs41_delegation_return(
+    IN nfs41_session *session,
+    IN nfs41_path_fh *file,
+    IN enum open_delegation_type4 access,
+    IN bool_t truncate)
+{
+    nfs41_client *client = session->client;
+    nfs41_delegation_state *deleg;
+    int status;
+
+    /* find a delegation for this file */
+    status = delegation_find(client, &file->fh, deleg_fh_cmp, &deleg);
+    if (status)
+        goto out;
+
+    AcquireSRWLockExclusive(&deleg->lock);
+    if (deleg->status == DELEGATION_GRANTED) {
+        /* return unless delegation is write and access is read */
+        if (deleg->state.type != OPEN_DELEGATE_WRITE
+            || access != OPEN_DELEGATE_READ) {
+            deleg->status = DELEGATION_RETURNING;
+            status = NFS4ERR_DELEG_REVOKED;
+        }
+    } else {
+        /* the delegation is being returned, wait for it to finish */
+        while (deleg->status != DELEGATION_RETURNING)
+            SleepConditionVariableSRW(&deleg->cond, &deleg->lock, INFINITE, 0);
+        status = NFS4ERR_BADHANDLE;
+    }
+    ReleaseSRWLockExclusive(&deleg->lock);
+
+    if (status == NFS4ERR_DELEG_REVOKED) {
+        delegation_return(client, deleg, truncate);
+        status = NFS4_OK;
+    }
+
+    nfs41_delegation_deref(deleg);
+out:
+    return status;
+}
+#endif
+
+
 /* asynchronous delegation recall */
 struct recall_thread_args {
     nfs41_client            *client;
@@ -390,17 +464,23 @@ int nfs41_delegation_recall(
     if (status)
         goto out;
 
-    /* start the delegation recall, or fail if it's already started */
+    AcquireSRWLockExclusive(&deleg->lock);
-    AcquireSRWLockShared(&deleg->lock);
+    if (deleg->state.recalled) {
-    if (deleg->state.recalled == 0)
+        /* return BADHANDLE if we've already responded to CB_RECALL */
-        deleg->state.recalled = 1;
-    else
         status = NFS4ERR_BADHANDLE;
-    ReleaseSRWLockShared(&deleg->lock);
+    } else {
-    if (status)
+        deleg->state.recalled = 1;
-        goto out_deleg;
 
-    /* TODO: return NFS4_OK if the delegation is already being returned */
+        if (deleg->status == DELEGATION_GRANTED) {
+            /* start the delegation return */
+            deleg->status = DELEGATION_RETURNING;
+            status = NFS4ERR_DELEG_REVOKED;
+        } /* else return NFS4_OK */
+    }
+    ReleaseSRWLockExclusive(&deleg->lock);
+
+    if (status != NFS4ERR_DELEG_REVOKED)
+        goto out_deleg;
 
     /* allocate thread arguments */
     args = calloc(1, sizeof(struct recall_thread_args));
@@ -422,6 +502,7 @@ int nfs41_delegation_recall(
         eprintf("nfs41_delegation_recall() failed to start thread\n");
         goto out_args;
     }
+    status = NFS4_OK;
 out:
     dprintf(DGLVL, "<-- nfs41_delegation_recall() returning %s\n",
         nfs_error_string(status));

daemon/delegation.h

@@ -27,6 +27,10 @@
 #include "nfs41.h"
 
 
+/* option to avoid conflicts by returning the delegation */
+#define DELEGATION_RETURN_ON_CONFLICT
+
+
 /* reference counting and cleanup */
 void nfs41_delegation_ref(
     IN nfs41_delegation_state *state);
@@ -59,6 +63,25 @@ int nfs41_delegation_to_open(
     IN bool_t try_recovery);
 
 
+/* synchronous delegation return */
+#ifdef DELEGATION_RETURN_ON_CONFLICT
+int nfs41_delegation_return(
+    IN nfs41_session *session,
+    IN nfs41_path_fh *file,
+    IN enum open_delegation_type4 access,
+    IN bool_t truncate);
+#else
+static int nfs41_delegation_return(
+    IN nfs41_session *session,
+    IN nfs41_path_fh *file,
+    IN enum open_delegation_type4 access,
+    IN bool_t truncate)
+{
+    return NFS4_OK;
+}
+#endif
+
+
 /* asynchronous delegation recall */
 int nfs41_delegation_recall(
     IN nfs41_client *client,

daemon/nfs41.h

@@ -80,13 +80,22 @@ typedef struct __nfs41_server {
     LONG ref_count;
 } nfs41_server;
 
+enum delegation_status {
+    DELEGATION_GRANTED,
+    DELEGATION_RETURNING,
+    DELEGATION_RETURNED,
+};
+
 typedef struct __nfs41_delegation_state {
     open_delegation4 state;
     nfs41_abs_path path;
     nfs41_path_fh file;
     struct list_entry client_entry; /* entry in nfs41_client.delegations */
     LONG ref_count;
+
+    enum delegation_status status;
     SRWLOCK lock;
+    CONDITION_VARIABLE cond;
 } nfs41_delegation_state;
 
 typedef struct __nfs41_lock_state {

daemon/open.c

@@ -133,7 +133,7 @@ void nfs41_open_stateid_arg(
     if (state->delegation.state) {
         nfs41_delegation_state *deleg = state->delegation.state;
         AcquireSRWLockShared(&deleg->lock);
-        if (!deleg->state.recalled) {
+        if (deleg->status == DELEGATION_GRANTED) {
             arg->type = STATEID_DELEG_FILE;
             memcpy(&arg->stateid, &deleg->state.stateid, sizeof(stateid4));
         }
@@ -668,6 +668,9 @@ static void cancel_open(IN nfs41_upcall *upcall)
 
     } else if (args->created) {
         const nfs41_component *name = &state->file.name;
+        /* break any delegations and truncate before REMOVE */
+        nfs41_delegation_return(state->session, &state->file,
+            OPEN_DELEGATE_WRITE, TRUE);
         status = nfs41_remove(state->session, &state->parent, name);
         if (status)
             dprintf(1, "cancel_open: nfs41_remove() failed with %s\n",
@@ -722,6 +725,10 @@ static int handle_close(nfs41_upcall *upcall)
             create_silly_rename(&state->path, &state->file.fh, name);
         }
 
+        /* break any delegations and truncate before REMOVE */
+        nfs41_delegation_return(state->session, &state->file,
+            OPEN_DELEGATE_WRITE, TRUE);
+
         dprintf(1, "calling nfs41_remove for %s\n", name->name);
         rm_status = nfs41_remove(state->session, &state->parent, name);
         if (rm_status) {

daemon/setattr.c

@@ -27,6 +27,7 @@
 
 #include "from_kernel.h"
 #include "nfs41_ops.h"
+#include "delegation.h"
 #include "name_cache.h"
 #include "upcall.h"
 #include "util.h"
@@ -93,8 +94,6 @@ static int handle_nfs41_setattr(setattr_upcall_args *args)
     nfs41_file_info info;
     int status = NO_ERROR;
 
-    nfs41_open_stateid_arg(state, &stateid);
-
     ZeroMemory(&info, sizeof(info));
 
     /* hidden */
@@ -145,6 +144,12 @@ static int handle_nfs41_setattr(setattr_upcall_args *args)
     if (!info.attrmask.count)
         goto out;
 
+    /* break read delegations before SETATTR */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_READ, FALSE);
+
+    nfs41_open_stateid_arg(state, &stateid);
+
     status = nfs41_setattr(state->session, &state->file, &stateid, &info);
     if (status) {
         dprintf(1, "nfs41_setattr() failed with error %s.\n",
@@ -160,6 +165,10 @@ static int handle_nfs41_remove(setattr_upcall_args *args)
     nfs41_open_state *state = args->state;
     int status;
 
+    /* break any delegations and truncate before REMOVE */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_WRITE, TRUE);
+
     status = nfs41_remove(state->session, &state->parent,
         &state->file.name);
     if (status)
@@ -216,7 +225,7 @@ static int handle_nfs41_rename(setattr_upcall_args *args)
     nfs41_session *dst_session;
     PFILE_RENAME_INFO rename = (PFILE_RENAME_INFO)args->buf;
     nfs41_abs_path dst_path;
-    nfs41_path_fh dst_dir;
+    nfs41_path_fh dst_dir, dst;
     nfs41_component dst_name, *src_name;
     uint32_t depth = 0;
     int status;
@@ -237,6 +246,10 @@ static int handle_nfs41_rename(setattr_upcall_args *args)
         create_silly_rename(&dst_path, &state->file.fh, &dst_name);
         dprintf(1, "silly rename: %s -> %s\n", src_name->name, dst_name.name);
 
+        /* break any delegations and truncate before silly rename */
+        nfs41_delegation_return(state->session, &state->file,
+            OPEN_DELEGATE_WRITE, TRUE);
+
         status = nfs41_rename(state->session,
             &state->parent, src_name,
             &dst_dir, &dst_name);
@@ -264,7 +277,7 @@ static int handle_nfs41_rename(setattr_upcall_args *args)
 
     /* the destination path is absolute, so start from the root session */
     status = nfs41_lookup(args->root, nfs41_root_session(args->root),
-        &dst_path, &dst_dir, NULL, NULL, &dst_session);
+        &dst_path, &dst_dir, &dst, NULL, &dst_session);
 
     while (status == ERROR_REPARSE) {
         if (++depth > NFS41_MAX_SYMLINK_DEPTH) {
@@ -294,6 +307,9 @@ static int handle_nfs41_rename(setattr_upcall_args *args)
             status = ERROR_FILE_EXISTS;
             goto out;
         }
+        /* break any delegations and truncate the destination file */
+        nfs41_delegation_return(dst_session, &dst,
+            OPEN_DELEGATE_WRITE, TRUE);
     } else if (status != ERROR_FILE_NOT_FOUND) {
         dprintf(1, "nfs41_lookup('%s') failed to find destination "
             "directory with %d\n", dst_path.path, status);
@@ -318,6 +334,10 @@ static int handle_nfs41_rename(setattr_upcall_args *args)
         goto out;
     }
 
+    /* break any delegations on the source file */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_WRITE, FALSE);
+
     status = nfs41_rename(state->session,
         &state->parent, src_name,
         &dst_dir, &dst_name);
@@ -343,6 +363,10 @@ static int handle_nfs41_set_size(setattr_upcall_args *args)
     nfs41_open_state *state = args->state;
     int status;
 
+    /* break read delegations before SETATTR */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_READ, FALSE);
+
     nfs41_open_stateid_arg(state, &stateid);
 
     ZeroMemory(&info, sizeof(info));
@@ -366,7 +390,7 @@ static int handle_nfs41_link(setattr_upcall_args *args)
     PFILE_LINK_INFORMATION link = (PFILE_LINK_INFORMATION)args->buf;
     nfs41_session *dst_session;
     nfs41_abs_path dst_path;
-    nfs41_path_fh dst_dir;
+    nfs41_path_fh dst_dir, dst;
     nfs41_component dst_name;
     uint32_t depth = 0;
     int status;
@@ -386,7 +410,7 @@ static int handle_nfs41_link(setattr_upcall_args *args)
 
     /* the destination path is absolute, so start from the root session */
     status = nfs41_lookup(args->root, nfs41_root_session(args->root),
-        &dst_path, &dst_dir, NULL, NULL, &dst_session);
+        &dst_path, &dst_dir, &dst, NULL, &dst_session);
 
     while (status == ERROR_REPARSE) {
         if (++depth > NFS41_MAX_SYMLINK_DEPTH) {
@@ -431,6 +455,10 @@ static int handle_nfs41_link(setattr_upcall_args *args)
     }
 
     if (status == NO_ERROR) {
+        /* break any delegations and truncate the destination file */
+        nfs41_delegation_return(dst_session, &dst,
+            OPEN_DELEGATE_WRITE, TRUE);
+
         /* LINK will return NFS4ERR_EXIST if the target file exists,
          * so we have to remove it ourselves */
         status = nfs41_remove(state->session, &dst_dir, &dst_name);
@@ -442,6 +470,10 @@ static int handle_nfs41_link(setattr_upcall_args *args)
         }
     }
 
+    /* break read delegations on the source file */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_READ, FALSE);
+
     status = nfs41_link(state->session, &state->file,
         &dst_dir, &dst_name, NULL);
     if (status) {
@@ -509,6 +541,10 @@ static int handle_setexattr(nfs41_upcall *upcall)
     stateid_arg stateid;
     nfs41_file_info info;
 
+    /* break read delegations before SETATTR */
+    nfs41_delegation_return(state->session, &state->file,
+        OPEN_DELEGATE_READ, FALSE);
+
     nfs41_open_stateid_arg(state, &stateid);
 
     ZeroMemory(&info, sizeof(info));