8:15 a.m.
On 5/14/19 7:19 AM, Madhu P Punjabi wrote:
We are seeing crash in svc_release_it().
We are using ganesha V2.7.3 and have taken some recent patches for ntirpc which includes
the patch to avoid leaking FDs "Mark ANYFD clients as local clients".
We are seeing the following crash repeatedly. Please check the backtrace below. Can there
be a chance of having the same 'xprt' getting destroyed twice
in clnt_vc_destroy()? Could this crash be related to it?
What do the logs say?
(gdb) l -
423 /* enforce once-only semantic, trace others */
424 xp_flags = atomic_postset_uint16_t_bits(&xprt->xp_flags,
425 SVC_XPRT_FLAG_RELEASING);
426 if (xp_flags & SVC_XPRT_FLAG_RELEASING) {
427 XPRT_TRACE(xprt, "WARNING! already destroying!", tag,
line);
428 return;
429 }
430
431 /* Releasing last reference */
432 (*(xprt)->xp_ops->xp_destroy)(xprt, flags, tag, line);
That atomic_postset should prevent two destroys. We used to have this problem all
the time (since before I'd joined in 2014), so I'd added that.
11:29 a.m.
The xprt->xp_refcnt is supposed to prevent two destroys in the first place.
If that isn't working correctly, then the atomic_postset is kinda useless
as it could be working on an unknown xprt memory while using the flag,
right?
On Tue, May 14, 2019 at 6:46 PM William Allen Simpson <
william.allen.simpson(a)gmail.com> wrote:
On 5/14/19 7:19 AM, Madhu P Punjabi wrote:
> We are seeing crash in svc_release_it().
> We are using ganesha V2.7.3 and have taken some recent patches for
ntirpc which includes the patch to avoid leaking FDs "Mark ANYFD clients as
local clients".
> We are seeing the following crash repeatedly. Please check the backtrace
below. Can there be a chance of having the same 'xprt' getting destroyed
twice in clnt_vc_destroy()? Could this crash be related to it?
What do the logs say?
> (gdb) l -
> 423 /* enforce once-only semantic, trace others */
> 424 xp_flags = atomic_postset_uint16_t_bits(&xprt->xp_flags,
> 425
SVC_XPRT_FLAG_RELEASING);
> 426 if (xp_flags & SVC_XPRT_FLAG_RELEASING) {
> 427 XPRT_TRACE(xprt, "WARNING! already destroying!",
tag, line);
> 428 return;
> 429 }
> 430
> 431 /* Releasing last reference */
> 432 (*(xprt)->xp_ops->xp_destroy)(xprt, flags, tag, line);
That atomic_postset should prevent two destroys. We used to have this
problem all
the time (since before I'd joined in 2014), so I'd added that.
_______________________________________________
Devel mailing list -- devel(a)lists.nfs-ganesha.org
To unsubscribe send an email to devel-leave(a)lists.nfs-ganesha.org
2:45 p.m.
On 5/15/19 12:29 PM, Malahal Naineni wrote:
The xprt->xp_refcnt is supposed to prevent two destroys in the
first place. If that isn't working correctly, then the atomic_postset is kinda useless
as it could be working on an unknown xprt memory while using the flag, right?
Obviously, that was the theory. But that flag helped find a lot of implementation holes
since 1994....
For one thing, the refs used to go to zero, then up to one again, and down to zero again
during the cleanup and free task.
Are they all fixed now? The Subject line seems to disagree.
10:05 p.m.
refs going up from zero is a tricky one to implement and hard to get it
right! I know we fixed few hashtable cases where refcount doesn't go up
from zero. I still see refs going up from zero in many places (even in
Ganesha code as well). So this could be one of them...
Regards, Malahal.
On Thu, May 16, 2019 at 1:15 AM William Allen Simpson <
william.allen.simpson(a)gmail.com> wrote:
On 5/15/19 12:29 PM, Malahal Naineni wrote:
> The xprt->xp_refcnt is supposed to prevent two destroys in the first
place. If that isn't working correctly, then the atomic_postset is kinda
useless as it could be working on an unknown xprt memory while using the
flag, right?
>
Obviously, that was the theory. But that flag helped find a lot of
implementation holes
since 1994....
For one thing, the refs used to go to zero, then up to one again, and down
to zero again
during the cleanup and free task.
Are they all fixed now? The Subject line seems to disagree.
9:21 a.m.
Sorry, I meant 2014. (I was thinking about the Usenix Security panel that I've been
invited to form, thus reading old security papers from 1989-1994.)
At the moment, I'm not in a position to help debug this. I'm not in Red
Hat's
Storage BU. I'm in upstream community and emerging technology under Red Hat's
CTO,
our lone "Open Standards" guru.
Since Matt Benjamin bypassed his boss, and went to my boss, and demanded that I stop
contributing to this project a year or so ago, I'm only around to help answer
questions about my code (mostly on my own time).
But bouncing refs from zero is not a good design, and I'd spent a lot of time
eliminating them. My preference is that they are initialized to 1 (in some cases 2)
during creation and first use, and only drop to zero once.
Fixing this stuff is often hard. Good luck.
On 5/15/19 11:05 PM, Malahal Naineni wrote:
refs going up from zero is a tricky one to implement and hard to get
it right! I know we fixed few hashtable cases where refcount doesn't go up from zero.
I still see refs going up from zero in many places (even in Ganesha code as well). So this
could be
one of them...
Regards, Malahal.
On Thu, May 16, 2019 at 1:15 AM William Allen Simpson <william.allen.simpson(a)gmail.com
<mailto:william.allen.simpson@gmail.com>> wrote:
On 5/15/19 12:29 PM, Malahal Naineni wrote:
> The xprt->xp_refcnt is supposed to prevent two destroys in the first place.
If that isn't working correctly, then the atomic_postset is kinda useless as it could
be working on an unknown xprt memory while using the flag, right?
>
Obviously, that was the theory. But that flag helped find a lot of implementation
holes
since 1994....
For one thing, the refs used to go to zero, then up to one again, and down to zero
again
during the cleanup and free task.
Are they all fixed now? The Subject line seems to disagree.
9:47 a.m.
xpt refcount has never been intended to drop to 0. Rather 1 (or 2 in
some code) was nominated as a sentinel refcount.
Matt
On Wed, May 15, 2019 at 11:05 PM Malahal Naineni <malahal(a)gmail.com> wrote:
refs going up from zero is a tricky one to implement and hard to get it right! I know we
fixed few hashtable cases where refcount doesn't go up from zero. I still see refs
going up from zero in many places (even in Ganesha code as well). So this could be one of
them...
Regards, Malahal.
On Thu, May 16, 2019 at 1:15 AM William Allen Simpson
<william.allen.simpson(a)gmail.com> wrote:
>
> On 5/15/19 12:29 PM, Malahal Naineni wrote:
> > The xprt->xp_refcnt is supposed to prevent two destroys in the first place.
If that isn't working correctly, then the atomic_postset is kinda useless as it could
be working on an unknown xprt memory while using the flag, right?
> >
>
> Obviously, that was the theory. But that flag helped find a lot of implementation
holes
> since 1994....
>
> For one thing, the refs used to go to zero, then up to one again, and down to zero
again
> during the cleanup and free task.
>
> Are they all fixed now? The Subject line seems to disagree.
_______________________________________________
Devel mailing list -- devel(a)lists.nfs-ganesha.org
To unsubscribe send an email to devel-leave(a)lists.nfs-ganesha.org
--
Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103
http://www.redhat.com/en/technologies/storage
tel. 734-821-5101
fax. 734-769-8938
cel. 734-216-5309
1816
days inactive
1818
days old
7 comments
4 participants
participants (4)
-
Madhu P Punjabi -
Malahal Naineni -
Matt Benjamin -
William Allen Simpson