Changes look good to me. I've ported them to illumos: http://reviews.csiden.org/r/46/diff/#index_header
I'll do some more testing and then put them up for a "real" illumos code review. Alexander, do you want to add any copyright notice to your patch before it goes to illumos? (it is not required) --matt On Sun, Jun 8, 2014 at 1:53 PM, Alexander Motin <[email protected]> wrote: > On 08.06.2014 21:32, Alexander Motin wrote: > >> On 07.06.2014 22:35, Matthew Ahrens wrote: >> >>> code review: >>> >>> zfsvfs_hold/rele need to pass the correct tags through to rrw_enter(). >>> The existing code will not work (if reference_tracking_enable), because >>> you will be holding with tag "zfsvfs_hold" and releasing with >>> "zfsvfs_rele". The existing code holds and releases with tag e.g. >>> "zfs_prop_set_userquota"; that needs to remain the case. >>> >> >> Indeed there was a bug, thank you. >> >> I think it would be cleaner to encapsulate the multitude of locks within >>> the rrw code, but I can live with it as is. >>> >> >> I had such thought initially, but then decided it will be too big mess >> to do it inside. Also it may not always be needed. But now you've asked, >> I've made intermediate solution: let me introduce Recursable Read-Mostly >> locks, as a wrapper on top of existing Recursable Read-Write locks. :) >> >> The attached patch implements rrmlock, and uses them for teardown lock. >> It indeed looks much better. Thanks for pushing me to do it. :) What do >> you think? >> > > I'm sorry, I've missed a bug until built kernel with debugging. Fixed > version is attached. > > -- > Alexander Motin >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
