George, we use a different vdev "driver" in FreeBSD, vdev_geom, but thank you for the idea. It seems that I can adapt it to vdev_geom.
On 16/09/2017 17:01, George Wilson wrote: > Andriy, > > Assuming this is disk specific, you could look at doing this in the IOCTL > callback -- "vdev_disk_ioctl_done". > > Thanks, > George > > On Fri, Sep 15, 2017 at 11:31 AM Andriy Gapon <[email protected] > <mailto:[email protected]>> wrote: > > > In FreeBSD we create a struct bio object for each I/O request that goes > to a > disk. So, we create bio-s for all vdev zio-s that are to be passed down > including read, write, ioctl (disk cache flush) zio-s. Previously, we > freed > those bio-s in a callback from the I/O subsystem. But since the ABD > change we > had to move that to the io_done stage, because the callback's context is > too > restrictive for abd_return_buf / abd_return_buf_copy (at least for the > current > ABD implementation which is taken as is from illumos). So, currently we > are > leaking bio-s created for ioctl zio-s because the io_done stage is not > called > for them. > > There is probably another way to fix this problem as the ioctl zio-s have > nothing to do with ABD. So, we could free their bio-s in the callback as > we did > before and handle the regular i/o bio-s in the io_done stage as we do > now. This > should work, but at the cost of the less uniform bio handling code. > > So, I wanted to see which of the solutions would be better from the point > of > view of the general zio pipeline architecture. > > Thank you. > > On 14/09/2017 18:48, George Wilson wrote: > > Andriy, > > > > The ZIO_STAGE_VDEV_IO_DONE is not necessary for ZIO_IOCTL_PIPELINE and > that's > > why it was removed. At least in illumos, there is no functional reason > for > > calling it. To add it back would require a small amount of change which > should > > not be a big deal. Can you provide some context around the FreeBSD bug? > > > > Thanks, > > George > > > > > > On Thu, Sep 14, 2017 at 12:45 AM Andriy Gapon <[email protected] > <mailto:[email protected]> > > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > > > Does anyone know why ZIO_IOCTL_PIPELINE does not include > ZIO_STAGE_VDEV_IO_DONE? > > Or, in other words, why ZIO_IOCTL_PIPELINE actively excludes it by > not > using > > ZIO_VDEV_IO_STAGES? > > > > It seems that ZIO_IOCTL_PIPELINE had ZIO_VDEV_IO_STAGES until this > commit: > > > commit e14bb3258d05c1b1077e2db7cf77088924e56919 > > > Author: Jeff Bonwick <[email protected]> > > > Date: Mon Sep 29 18:13:58 2008 -0700 > > > 6754011 SPA 3.0: lock breakup, i/o pipeline refactoring, device > failure > > handling > > > 6667208 zfs/zpool commands on failed pool should not hang > > > 6430480 grabbing config lock as writer during I/O load can take > > excessively long > > > > Of course, the commit message does not have any detailed > explanations > and the > > referenced bugs are not publicly accessible. And it was almost 9 > years ago. > > > > I wonder if the change was because of anything specific to illumos > vdev_disk. > > I think that it would be totally okay to enable > ZIO_STAGE_VDEV_IO_DONE > with > > FreeBSD vdev_geom. In fact, right now ZFS/FreeBSD has a bug because > the done > > stage is not executed. > > > > -- > > Andriy Gapon > > > > ------------------------------------------ > > illumos-zfs > > Archives: > > > > https://illumos.topicbox.com/groups/zfs/discussions/T7be335e1da154b96-Mea10ec00d17c40849ece338f > > Powered by Topicbox: https://topicbox.com > > > > *illumos-zfs* | Archives > > > > <https://illumos.topicbox.com/groups/zfs/discussions/T7be335e1da154b96-M0cc0a74989d1c00edb4b391a> > > | Powered by Topicbox <https://topicbox.com> > > > -- > Andriy Gapon > -- Andriy Gapon ------------------------------------------ openzfs-developer Archives: https://openzfs.topicbox.com/groups/developer/discussions/T4b8a967c63e22769-M6e632b28aa871ba0b85be0a6 Powered by Topicbox: https://topicbox.com
