Vendor merge review: ZFS I/O deadman thread
Davide Italiano
davide at freebsd.org
Sat Feb 23 13:44:25 UTC 2013
On Sat, Feb 23, 2013 at 1:57 PM, Martin Matuska <mm at freebsd.org> wrote:
> Please review the following proposed merge from vendor.
>
> The patch uses callout_drain(), I am not sure if callout_stop() is safe in this case.
Assuming you want to maintain the same semantic as Illumos code, and
also assuming the Illumos code is correct, I think the right choice
here is callout_drain().
In fact, if I understand correctly the changes, Illumos code rely on
cyclic_remove(), which guarantees that the removed cyclic handler has
completed execution (blocking, if necessary).
Also, I see the code rely on cyclic which provides high-resolution
timers rather than coarse-grained callout. I'm not really sure if you
need such high granularity, but in case do, you could consider
migrating the patch to the new callout_* functions once they're
committed (hopefully, next week or something like).
> As of the patch, the deadman thread is disabled by default. Should we enable it?
>
> Thank you.
>
> http://people.freebsd.org/~mm/patches/zfs/zfs_deadman.patch
>
> Link to vendor ZFS issue:
> 3246 ZFS I/O deadman thread
> https://www.illumos.org/issues/3246
>
> --
> Martin Matuska
> FreeBSD committer
> http://blog.vx.sk
>
>
> _______________________________________________
> zfs-devel at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/zfs-devel
> To unsubscribe, send any mail to "zfs-devel-unsubscribe at freebsd.org"
Thanks,
Davide
More information about the zfs-devel
mailing list