Re: [Nfs-ganesha-devel] nTI-RPC refcounting and locking
It sounds like your change is a step in the direction of unifying CLNT and SVCXPRT handle structures. As we've discussed off-list, if you take on the project of unifying the actual handle structures, you get the lock consolidation for free. In any event, if rpc_dplx_rec contains a service handle expanded, it appears to need a client handle as well. Matt - Original Message - > From: "William Allen Simpson" <william.allen.simp...@gmail.com> > To: "NFS Ganesha Developers" <nfs-ganesha-devel@lists.sourceforge.net>, "Swen > Schillig" <s...@vnet.ibm.com> > Sent: Monday, January 16, 2017 1:53:24 PM > Subject: [Nfs-ganesha-devel] nTI-RPC refcounting and locking > > Swen, I've been looking at your patch, and it has some good ideas. > For some odd reason, I awoke at 1:30 am thinking about it, and > got up and wrote some code. > > I've taken another patch of mine, and added the SVCXPRT into the > rpc_dplx_rec, eliminating the refcnt entirely (using the SVCXPRT). > > After all, there's no reason to zalloc them separately. They > always are created at the same time. > > So I'm wondering about your thoughts on the locking. They seem > redundant. I'm thinking about changing REC_LOCK to use the > SVCXPRT xp_lock, instead. > > There's a spot in the existing rpc_dplx_rec creation code where > there's a timing hole in the code after an existing one is > found so the extra refcount is decremented. Another process > could also decrement and free, and there could be a pointer into > freed memory. Unifying the lock would be one solution (better > and faster than the usual solution with two locks). > > The SVCXPRT lock code has a lot more debugging and testing, too. > > Any other related ideas? > > BTW, I got rid of the , too. Changed it to a callback > function ;) > > -- > Check out the vibrant tech community on one of the world's most > engaging tech sites, SlashDot.org! http://sdm.link/slashdot > ___ > Nfs-ganesha-devel mailing list > Nfs-ganesha-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel > -- 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 -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
Re: [Nfs-ganesha-devel] nTI-RPC refcounting and locking
On Mo, 2017-01-16 at 13:53 -0500, William Allen Simpson wrote: > Swen, I've been looking at your patch, and it has some good ideas. > For some odd reason, I awoke at 1:30 am thinking about it, and > got up and wrote some code. > I never intended to give you sleepless nights :-) > I've taken another patch of mine, and added the SVCXPRT into the > rpc_dplx_rec, eliminating the refcnt entirely (using the SVCXPRT). > > After all, there's no reason to zalloc them separately. They > always are created at the same time. > > So I'm wondering about your thoughts on the locking. They seem > redundant. I'm thinking about changing REC_LOCK to use the > SVCXPRT xp_lock, instead. > > There's a spot in the existing rpc_dplx_rec creation code where > there's a timing hole in the code after an existing one is > found so the extra refcount is decremented. Another process > could also decrement and free, and there could be a pointer into > freed memory. Unifying the lock would be one solution (better > and faster than the usual solution with two locks). > > The SVCXPRT lock code has a lot more debugging and testing, too. > > Any other related ideas? > > BTW, I got rid of the , too. Changed it to a callback > function ;) That sounds like a much more invasive change. I wasn't that brave at the time and just tried to fix what was wrong, anyhow, a good rewrite of that area is always favorable. It would be good if you could post/include all your patches as soon as possible as I believe the ntirpc area does need some updates. I hope I will find some time again soon and try to help out as well, if that's ok. Cheers Swen -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
[Nfs-ganesha-devel] nTI-RPC refcounting and locking
Swen, I've been looking at your patch, and it has some good ideas. For some odd reason, I awoke at 1:30 am thinking about it, and got up and wrote some code. I've taken another patch of mine, and added the SVCXPRT into the rpc_dplx_rec, eliminating the refcnt entirely (using the SVCXPRT). After all, there's no reason to zalloc them separately. They always are created at the same time. So I'm wondering about your thoughts on the locking. They seem redundant. I'm thinking about changing REC_LOCK to use the SVCXPRT xp_lock, instead. There's a spot in the existing rpc_dplx_rec creation code where there's a timing hole in the code after an existing one is found so the extra refcount is decremented. Another process could also decrement and free, and there could be a pointer into freed memory. Unifying the lock would be one solution (better and faster than the usual solution with two locks). The SVCXPRT lock code has a lot more debugging and testing, too. Any other related ideas? BTW, I got rid of the , too. Changed it to a callback function ;) -- Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot ___ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
