On 12/3/13 9:49 PM, Saso Kiselkov wrote:
On 12/4/13, 2:19 AM, Matthew Ahrens wrote:
On Fri, Nov 29, 2013 at 1:36 PM, Saso Kiselkov <skiselkov...@gmail.com
<mailto:skiselkov...@gmail.com>> wrote:

     The only "good" reason I can think of is read-write locks not being
     present in the kernel when this was written.


rwlocks definitely existed (in the solaris kernel) when ZFS was written.
  I'm not sure why we used a hand-rolled reader-writer lock rather than a
krwlock_t.  Maybe George knows?
I suspect that the problem could be related with not wanting to acquire
and subsequently drop a rwlock in different threads?

     Other than that, the
     spa_config_lock semantics are exactly the same as in krwlock_t.

     > Is there some kind of lock debugging facility in illumos kernel?

     You'll have to be a bit more specific. There's plenty of lock analysis
     and debugging tools, but that's really not what we're after here. We're
     after the semantics. I think I understand the krwlock_t semantics from
     looking at the code (and they appear identical to FreeBSD's, though I
     haven't looked at Linux), and as far as I can discern, the code in that
     webrev gives the exact same behavior as the legacy spa_config_locks
     implementation.


Using a rwlock would lose the debugability of the refcount_t scl_count
(tag matching, ::refcount).  That may be acceptable, though.
Well, according to Alex, it does improve performance (though we have to
see more details on that still) and it simplifies the code a lot.

Cheers,
Sorry I'm coming late to the party. The reason for the special spa_config_lock semantics is to ensure that a lock can be grabbed by one thread and released by another. Changing to a plain rwlock implementation would be problematic based on that requirement. I believe that this is only an issue when you grab the rwlock as writer since it tracks the thread that obtained the write lock in the locking structure. I don't recall if we still have instances where we grab the lock as writer and then release it by a different thread so I think it makes sense to investigate making the change to a plain rwlock implementation. This would require an extensive amount of testing especially when dealing with configuration changes and cases where we deal with I/O errors (vdev_probe code path).

Thanks,
George

_______________________________________________
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer

Reply via email to