prakashsurya commented on this pull request.
- ASSERT(zilog->zl_root_zio == NULL); + ASSERT(MUTEX_HELD(&zilog->zl_writer_lock)); I'm not sure I understand. The `zl_writer_lock` and `zl_lock` are used for different purposes. The `zl_writer_lock` is used solely to ensure only a single thread creates and issues an "lwb" at any given time. Thus, we acquire it in only a couple places; in `zil_commit_writer` and `zil_commit_waiter_timeout`. The `zl_lock` is acquired in many different places, and it's OK to acquire the `zl_lock` after already holding the `zl_writer_lock`. The locking you see here is by design. With that said, I'm not particularly happy with the naming scheme. The `zl_lock` name was in place prior to this patch, and I couldn't think of a descriptive name that would be any better at describing what it is protecting. If the comment is more about how the locking in the ZIL is tricky and hard to understand, then I definitely agree. I tried to make it better (not sure if I succeeded in that), but I purposely did not try to change much of the locking surrounding the `zl_lock` and what it protects. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/447#discussion_r136596856 ------------------------------------------ openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/Tb8f4a0d0bed9b5eb-M68219099c18b01a9e58b048a Powered by Topicbox: https://topicbox.com
