- only compiled, not run-tested
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
While I can't say anything bad about this change, I think we should better follow ZoL who added taskq_dispatch_delay() in addition to taskq_dispatch(), backed by their equivalent of taskqueue_enqueue_timeout(). In this particular case your patch is probably more efficient. but it is not a testing path, so should not care about extra context switch more then code divergence.
Just to chime in with what @mav said. Once FreeBSD is merged with the soon to be OpenZFS repo, the missing features (rename -u, FreeBSD's ashift mechanism, etc) have been upstreamed and the port has been given a few months to soak - the current code in tree - based as it is on a near dead upstream - is going away.
Take a look at the current working branch:
https://github.com/zfsonfreebsd/ZoF/tree/projects/pr-rebase-20191128/module
The change looks good to me.
Regarding other topics mentioned, I do not think that the code in stable branches is going away soon.
Also, while I am hopeful that the ZoF code lands before FreeBSD 13, I do not see any harm in improving the current ZFS code before that.
And having plan B is always nice.
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c | ||
---|---|---|
730 ↗ | (On Diff #65021) | Mmm, actually I think that we should assert here that the callout is not pending. That is, it was never scheduled or has already completed. There should be no way to get here if the zio has not finished its pipeline. |
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c | ||
---|---|---|
662 ↗ | (On Diff #65021) | This needs to go under #ifdef _KERNEL, just like body of zio_delay_interrupt(), since the file is also compiled in userland. I've done the same patch couple days ago :) |
For the purposes of retiring the old timeout_t, I'd rather use a smaller diff. I looked at the ZoF tree, but there's no instance of 'taskq_dispatch_delay' in zfs/zio.c (which I assumed was the same as this file), so I didn't see a way to extract a simple diff that would apply easily to the current ZFS code.