From 006bdfa47af1c3af5506aa3979365a0f0879af07 Mon Sep 17 00:00:00 2001 From: Olga Kornievskaia Date: Wed, 27 Oct 2010 16:18:12 -0400 Subject: [PATCH] ref counting for nfs41_open_state --- daemon/getattr.c | 1 + daemon/lock.c | 2 ++ daemon/nfs41.h | 15 +++++++++++++++ daemon/open.c | 41 ++++++++++++++++++++++++++++++++++++----- daemon/readdir.c | 1 + daemon/readwrite.c | 1 + daemon/setattr.c | 2 ++ daemon/symlink.c | 1 + daemon/upcall.c | 5 +++++ daemon/upcall.h | 14 ++++++++++++++ 10 files changed, 78 insertions(+), 5 deletions(-) diff --git a/daemon/getattr.c b/daemon/getattr.c index 1200eb5..30ee53e 100644 --- a/daemon/getattr.c +++ b/daemon/getattr.c @@ -71,6 +71,7 @@ static int parse_getattr(unsigned char *buffer, uint32_t length, nfs41_upcall *u if (status) goto out; status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); dprintf(1, "parsing NFS41_FILE_QUERY: info_class=%d buf_len=%d " "root=0x%p open_state=0x%p\n", diff --git a/daemon/lock.c b/daemon/lock.c index 69d632f..c3476bf 100644 --- a/daemon/lock.c +++ b/daemon/lock.c @@ -94,6 +94,7 @@ static int parse_lock(unsigned char *buffer, uint32_t length, nfs41_upcall *upca status = safe_read(&buffer, &length, &args->state, sizeof(HANDLE)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); status = safe_read(&buffer, &length, &args->root, sizeof(HANDLE)); if (status) goto out; status = safe_read(&buffer, &length, &args->offset, sizeof(LONGLONG)); @@ -181,6 +182,7 @@ static int parse_unlock(unsigned char *buffer, uint32_t length, nfs41_upcall *up status = safe_read(&buffer, &length, &args->state, sizeof(HANDLE)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); status = safe_read(&buffer, &length, &args->root, sizeof(HANDLE)); if (status) goto out; status = safe_read(&buffer, &length, &args->count, sizeof(ULONG)); diff --git a/daemon/nfs41.h b/daemon/nfs41.h index 3bbcb6f..930079f 100644 --- a/daemon/nfs41.h +++ b/daemon/nfs41.h @@ -72,6 +72,12 @@ typedef struct __nfs41_lock_state { SRWLOCK lock; } nfs41_lock_state; +/* nfs41_open_state reference counting: + * one reference is held implicitly by the driver (initialized to 1 on + * OPEN and released on CLOSE). other references must be held during + * upcalls to prevent a parallel CLOSE from freeing it prematurely. by + * calling upcall_open_state_ref() when parsing the upcall, you are + * guaranteed a matching dereference on upcall_cleanup() */ typedef struct __nfs41_open_state { nfs41_abs_path path; nfs41_path_fh parent; @@ -85,6 +91,7 @@ typedef struct __nfs41_open_state { nfs41_lock_state last_lock; struct __pnfs_file_layout *layout; SRWLOCK lock; + LONG ref_count; } nfs41_open_state; typedef struct __nfs41_rpc_clnt { @@ -355,4 +362,12 @@ static __inline netaddr4* nfs41_rpc_netaddr( bool_t nfs41_renew_in_progress(nfs41_client *client, bool_t *value); + +/* open.c */ +void nfs41_open_state_ref( + IN nfs41_open_state *state); + +void nfs41_open_state_deref( + IN nfs41_open_state *state); + #endif /* __NFS41__ */ diff --git a/daemon/open.c b/daemon/open.c index f214d98..e0bf833 100644 --- a/daemon/open.c +++ b/daemon/open.c @@ -61,6 +61,7 @@ static int create_open_state( "%u", open_owner_id); state->owner.owner_len = (uint32_t)strlen( (const char*)state->owner.owner); + state->ref_count = 1; *state_out = state; status = NO_ERROR; @@ -73,13 +74,33 @@ out_free: } static void free_open_state( - IN nfs41_session *session, IN nfs41_open_state *state) { free(state); } +void nfs41_open_state_ref( + IN nfs41_open_state *state) +{ + const LONG count = InterlockedIncrement(&state->ref_count); + + dprintf(2, "nfs41_open_state_ref(%s) count %d\n", + state->path.path, count); +} + +void nfs41_open_state_deref( + IN nfs41_open_state *state) +{ + const LONG count = InterlockedDecrement(&state->ref_count); + + dprintf(2, "nfs41_open_state_deref(%s) count %d\n", + state->path.path, count); + if (count == 0) + free_open_state(state); +} + + /* NFS41_OPEN */ static int parse_open(unsigned char *buffer, uint32_t length, nfs41_upcall *upcall) { @@ -398,7 +419,7 @@ static int handle_open(nfs41_upcall *upcall) out: return status; out_free_state: - free(state); + nfs41_open_state_deref(state); goto out; } @@ -461,7 +482,7 @@ static void cancel_open(IN nfs41_upcall *upcall) nfs_error_string(status)); } - free_open_state(state->session, state); + nfs41_open_state_deref(state); out: status = nfs_to_windows_error(status, ERROR_INTERNAL_ERROR); dprintf(1, "<-- cancel_open() returning %d\n", status); @@ -531,13 +552,20 @@ static int handle_close(nfs41_upcall *upcall) } } - free_open_state(state->session, state); if (status || !rm_status) return status; else return rm_status; } +static void cleanup_close(nfs41_upcall *upcall) +{ + close_upcall_args *args = &upcall->args.close; + + /* release the initial reference from create_open_state() */ + nfs41_open_state_deref(args->state); +} + const nfs41_upcall_op nfs41_op_open = { parse_open, @@ -547,5 +575,8 @@ const nfs41_upcall_op nfs41_op_open = { }; const nfs41_upcall_op nfs41_op_close = { parse_close, - handle_close + handle_close, + NULL, + NULL, + cleanup_close }; diff --git a/daemon/readdir.c b/daemon/readdir.c index 7a6a5b9..9e81474 100644 --- a/daemon/readdir.c +++ b/daemon/readdir.c @@ -64,6 +64,7 @@ static int parse_readdir(unsigned char *buffer, uint32_t length, nfs41_upcall *u if (status) goto out; status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); dprintf(1, "parsing NFS41_DIR_QUERY: info_class=%d buf_len=%d " "filter='%s'\n\tInitial\\Restart\\Single %d\\%d\\%d " diff --git a/daemon/readwrite.c b/daemon/readwrite.c index 438ec86..615931b 100644 --- a/daemon/readwrite.c +++ b/daemon/readwrite.c @@ -49,6 +49,7 @@ static int parse_rw(unsigned char *buffer, uint32_t length, nfs41_upcall *upcall if (status) goto out; status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); dprintf(1, "parsing %s len=%ld offset=%ld buf=%p root=%p " "open_state=0x%p\n", opcode2string(upcall->opcode), args->len, diff --git a/daemon/setattr.c b/daemon/setattr.c index d3e45ff..b2ab812 100644 --- a/daemon/setattr.c +++ b/daemon/setattr.c @@ -56,6 +56,7 @@ static int parse_setattr(unsigned char *buffer, uint32_t length, nfs41_upcall *u if (status) goto out_free; status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); if (status) goto out_free; + upcall_open_state_ref(upcall, args->state); status = safe_read(&buffer, &length, &args->open_owner_id, sizeof(ULONG)); if (status) goto out_free; status = safe_read(&buffer, &length, &args->access_mask, sizeof(ULONG)); @@ -501,6 +502,7 @@ static int parse_setexattr(unsigned char *buffer, uint32_t length, nfs41_upcall if (status) goto out; status = safe_read(&buffer, &length, &args->state, sizeof(args->state)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); status = safe_read(&buffer, &length, &args->mode, sizeof(args->mode)); if (status) goto out; diff --git a/daemon/symlink.c b/daemon/symlink.c index b3d53f5..94240d8 100644 --- a/daemon/symlink.c +++ b/daemon/symlink.c @@ -199,6 +199,7 @@ static int parse_symlink(unsigned char *buffer, uint32_t length, nfs41_upcall *u if (status) goto out; status = safe_read(&buffer, &length, &args->state, sizeof(nfs41_open_state *)); if (status) goto out; + upcall_open_state_ref(upcall, args->state); status = get_name(&buffer, &length, &args->path); if (status) goto out; status = safe_read(&buffer, &length, &args->set, sizeof(BOOLEAN)); diff --git a/daemon/upcall.c b/daemon/upcall.c index edd4928..fc16d83 100644 --- a/daemon/upcall.c +++ b/daemon/upcall.c @@ -181,4 +181,9 @@ void upcall_cleanup( const nfs41_upcall_op *op = g_upcall_op_table[upcall->opcode]; if (op && op->cleanup) op->cleanup(upcall); + + if (upcall->state_ref) { + nfs41_open_state_deref(upcall->state_ref); + upcall->state_ref = NULL; + } } diff --git a/daemon/upcall.h b/daemon/upcall.h index 62f91d6..b071fa6 100644 --- a/daemon/upcall.h +++ b/daemon/upcall.h @@ -176,6 +176,11 @@ typedef struct __nfs41_upcall { uint32_t status; uint32_t last_error; upcall_args args; + + /* store referenced pointers with the upcall for + * automatic dereferencing on upcall_cleanup(); + * see upcall_root_ref() and upcall_open_state_ref() */ + nfs41_open_state *state_ref; } nfs41_upcall; @@ -216,4 +221,13 @@ void upcall_cancel( void upcall_cleanup( IN nfs41_upcall *upcall); + +static __inline void upcall_open_state_ref( + IN nfs41_upcall *upcall, + IN nfs41_open_state *state) +{ + nfs41_open_state_ref(state); + upcall->state_ref = state; +} + #endif /* !__NFS41_DAEMON_UPCALL_H__ */