1701 ZFS to support UNMAP/TRIM for SSD

Review Request #2278 — Created Sept. 3, 2019 and submitted

jjelinek
illumos-gate
1701
general

Port from ZoL of TRIM support. Some notable changes from the port:
usr/src/test/zfs-tests/cmd/has_unmap/has_unmap.c
I wrote this and hooked in to new trim test setup scripts
usr/src/uts/common/fs/zfs/sys/vdev_impl.h
added vdev_autotrim_bytes_done for use in test validation
usr/src/test/zfs-tests/tests/functional/trim/trim.kshlib
added get_illumos_trim_io() to validate autotrim progress
usr/src/uts/common/fs/zfs/vdev_disk.c
vdev_disk_open() changed to use DKIOC_CANFREE
vdev_disk_io_start() cahnged for ZIO_TYPE_TRIM case to use DKIOCFREE

zfs-test run including new trim tests

  • 0
  • 0
  • 8
  • 0
  • 8
Description From Last Updated
seeemef@mac.com
  1. Looks good. Just found a few comment nits.

  2. typo "initialing"?

  3. usr/src/uts/common/fs/zfs/vdev_trim.c (Diff revision 1)
     
     

    missing hyphen for "in flight"?

  4. usr/src/uts/common/fs/zfs/vdev_trim.c (Diff revision 1)
     
     

    missing hyphen for "in flight"?

  5. usr/src/uts/common/fs/zfs/vdev_trim.c (Diff revision 1)
     
     

    missing hyphen for "in flight"?

  6. usr/src/uts/common/fs/zfs/vdev_trim.c (Diff revision 1)
     
     

    typo "in to" for "into"?

  7. usr/src/uts/common/fs/zfs/zfs_ioctl.c (Diff revision 1)
     
     

    typo plural "commands"?

  8. usr/src/uts/common/fs/zfs/zfs_ioctl.c (Diff revision 1)
     
     

    typo plural "commands"?

  9. 
      
kkantor
  1. Looks good! I can +1 this if you think we don't need to add the -r flag to the zpool man page.

  2. usr/src/lib/libzfs/common/libzfs.h (Diff revision 1)
     
     

    Is there a reason this needed to be moved? I don't mind the move, I'm just curious if there's a binary compat problem.

    1. There is no binary compatibility problem. I moved it to match ZoL so that it would stop causing spurious conflicts when we try to apply ZoL patches.

  3. usr/src/man/man1m/zpool.1m (Diff revision 1)
     
     

    the -r flag isn't in the flag list here. Should it be? It looks like it's missing in ZoL too.

  4. usr/src/uts/common/fs/zfs/sys/spa.h (Diff revision 1)
     
     

    It looks like this isn't used yet. I should really make the time to revisit the ZoL stat additions.

  5. usr/src/uts/common/fs/zfs/vdev_queue.c (Diff revision 1)
     
     

    Does a linter complain about this branch being empty on non-debug bits?

    1. No, smatch is not complaining.

  6. 
      
jjelinek
kkantor
  1. Ship It!
  2. 
      
seeemef@mac.com
  1. Ship It!
  2. 
      
jjelinek
Review request changed

Status: Closed (submitted)

Loading...