[M] Change in ...nfs-ganesha[next]: Multi-client same hostname issue - open owner refcount fix
by Name of user not set (GerritHub)
deepakarumugam.s(a)nutanix.com has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553739?usp=email )
Change subject: Multi-client same hostname issue - open owner refcount fix
......................................................................
Multi-client same hostname issue - open owner refcount fix
This is a followup to
https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553234
Issue/Bug:
After cherry-picking the changes our code flow for
creating a nfs4 open owner looks like this
(1) owner.refcount = 1
(memcpy from key to owner accomplishes this)
(2) mutex_lock(clid_mutex)
add owner to clid_list
mutex_unlock(clid_mutex)
(3) owner.refcount = 1 (again)
The problem is when the client id expiration code
gets interleaved between (2) and (3) it does
something like this,
mutex_lock(clid_mutex)
atomic_inc(owner.refcount) // value goes up to 2
mutex_unlock(clid_mutex)
Now when step (3) gets executed after the
client id expiration code it resets refcount
from 2 to 1.
This causes unnecessary freeing of open_owner
structures and lots of crashes segfaults etc.
Solution:
We remove (3) (ie) resetting refcount to 1
after adding it to the list.
This code is also used for nlm owner
initializations. nlm code assumes that
get_state_owner takes care of the refcount
so now we have to take care of the refcount
in nlm callback
Change-Id: I8ed40adee9b60fb3101aafb1b675c7c782a1f640
Signed-off-by: Deepak Arumugam Sankara Subramanian <deepakarumugam.s(a)nutanix.com>
---
M src/SAL/nlm_owner.c
M src/SAL/state_misc.c
2 files changed, 50 insertions(+), 2 deletions(-)
git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/39/553739/1
--
To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553739?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: I8ed40adee9b60fb3101aafb1b675c7c782a1f640
Gerrit-Change-Number: 553739
Gerrit-PatchSet: 1
Gerrit-Owner: deepakarumugam.s(a)nutanix.com
Gerrit-MessageType: newchange
11 months, 3 weeks
[S] Change in ...nfs-ganesha[next]: NLM: Fix NSM reference count double-decrement
by Martin Schwenke (GerritHub)
Martin Schwenke has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553524?usp=email )
Change subject: NLM: Fix NSM reference count double-decrement
......................................................................
NLM: Fix NSM reference count double-decrement
Fixes: https://github.com/nfs-ganesha/nfs-ganesha/issues/937
Commit 5febadaa98fb53bc4c3f2dab7793d6afb3847073 moves the NSM
reference count decrement before a possible error is handled.
Commit 275539c02ad3984e17d6d78b494bd9328cf466c7 (authored before the
above, merged after) causes the old sm_unmonitor() function (renamed
to nsm_unmonitor_no_retry()) to be called twice if the first attempt
fails. This means the reference count is likely to be decremented
twice on error.
The commit message for 5febadaa98fb53bc4c3f2dab7793d6afb3847073 says
the reference count decrement needs to be before the call to
nsm_disconnect(). However, it was already before the call to
nsm_disconnect() in the successful case, and it doesn't seem sane to
decrement the reference count on failure. Therefore, partially
reverting 5febadaa98fb53bc4c3f2dab7793d6afb3847073 by moving the
reference count back to where it was.
Signed-off-by: Martin Schwenke <mschwenke(a)ddn.com>
Change-Id: I595e5193f945fe7bcb30cb05d7e2de31f69b7920
---
M src/Protocols/NLM/nsm.c
1 file changed, 30 insertions(+), 1 deletion(-)
git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/24/553524/1
--
To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553524?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: I595e5193f945fe7bcb30cb05d7e2de31f69b7920
Gerrit-Change-Number: 553524
Gerrit-PatchSet: 1
Gerrit-Owner: Martin Schwenke <martin(a)meltin.net>
Gerrit-MessageType: newchange
11 months, 4 weeks
Broken reference count in NSM retry?
by Martin Schwenke
I came across this while we were trying to debug
https://github.com/nfs-ganesha/nfs-ganesha/issues/680 (not quite there
yet).
The following commit was merged fairly recently, partly at my request:
commit 275539c02ad3984e17d6d78b494bd9328cf466c7
Author: Malahal Naineni <malahal(a)us.ibm.com>
AuthorDate: Wed Oct 24 14:04:34 2018 +0530
Commit: Frank S. Filz <ffilzlnx(a)mindspring.com>
CommitDate: Fri Nov 18 10:33:13 2022 -0800
Retry communication with NSM service
It retries SM_MON and SM_UNMON on failure, forcing a disconnect on
error.
The above commit was written before the following one, but was merged
after:
commit 5febadaa98fb53bc4c3f2dab7793d6afb3847073
Author: Gaurav B. Gangalwar <gaurav.gangalwar(a)gmail.com>
AuthorDate: Tue Feb 19 10:42:57 2019 -0500
Commit: Frank S. Filz <ffilzlnx(a)mindspring.com>
CommitDate: Fri Feb 22 10:24:07 2019 -0800
Reduce nsm_count before nsm_disconnect as we will not be able to disconnect nsm
client in case of RPC failures.
Change-Id: I0cd5f0614e89f447dc7c5ac278cf03224add05e2
Signed-off-by: Gaurav B. Gangalwar <gaurav.gangalwar(a)gmail.com>
It moves nsm_count-- before the last error check.
The combination of both patches means that, on error, an SM_UNMON
request can reduce nsm_count by 2, making it negative. Oops!
The 2nd commit claims to "Reduce nsm_count before nsm_disconnect".
However, when you look at the patch and the code, the call to
nsm_disconnect() is after the final LogDebug() in the patch context, so
nsm_count-- was always before the call to nsm_disconnect(). So, unless
there's a something subtle going on (e.g. it really wants to decrement
the reference count before the call to clnt_req_release()) then the
patch doesn't do what the commit message says.
In my mind, the simplest solution to the double-decrement-on-retry
issue is to move nsm_count-- back to where it was before, so it is only
done on success.
If that seems sane then I'm happy to submit a patch doing that.
Thanks!
peace & happiness,
martin
11 months, 4 weeks
[M] Change in ...nfs-ganesha[next]: monitoring: C++ cleanup to build with userspace-rcu-14.0.0
by Kaleb KEITHLEY (GerritHub)
Kaleb KEITHLEY has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553407?usp=email )
Change subject: monitoring: C++ cleanup to build with userspace-rcu-14.0.0
......................................................................
monitoring: C++ cleanup to build with userspace-rcu-14.0.0
New version of userspace-rcu has now has broad c++ support and the
original monitoring implementation fails to build as a result of the
use nested 'extern "C" { ... }'. Note that nested 'extern "C"' is
tricky at best to use.
This patch adds safe, non-nested extern "C" protection to the minimal
set of ganesha internal headers necessary to build monitoring. The one
hack here is the wrapping of the
#include <rpc/xdr_inline.h>
in include/gsh_rpc.h. gsh_rpc is one of just a couple of ntirpc headers
that does not have its own
'#ifdef __cplusplus extern "C" { ... } #endif'
wrapper.
Change-Id: I680ad2c79648ef44c54ad43e653db113722b66a9
ChangeId: I5834b8061f01569d1d51f5882739c23d7072cf1a
Signed-off-by: Kaleb S. KEITHLEY <kkeithle(a)redhat.com>
---
M src/include/config_parsing.h
M src/include/display.h
M src/include/gsh_list.h
M src/include/gsh_refstr.h
M src/include/gsh_rpc.h
M src/include/gsh_types.h
M src/include/log.h
M src/include/monitoring.h
M src/include/mount.h
M src/include/nfs23.h
M src/include/nfs_convert.h
M src/monitoring/monitoring.cc
12 files changed, 122 insertions(+), 2 deletions(-)
git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/07/553407/1
--
To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553407?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings
Gerrit-Project: ffilz/nfs-ganesha
Gerrit-Branch: next
Gerrit-Change-Id: I680ad2c79648ef44c54ad43e653db113722b66a9
Gerrit-Change-Number: 553407
Gerrit-PatchSet: 1
Gerrit-Owner: Kaleb KEITHLEY <kaleb(a)redhat.com>
Gerrit-MessageType: newchange
12 months