11714 temporarily disable ZFS TRIM support

Review Request #2318 — Created Sept. 19, 2019 and submitted

jjelinek
illumos-gate
11714
general

Disable trim by default.



  • 0
  • 0
  • 0
  • 2
  • 2
Description From Last Updated
tsoome
  1. Ship It!
  2. 
      
igork
  1. 
      
  2. usr/src/uts/common/fs/zfs/vdev_disk.c (Diff revision 1)
     
     

    will be better to use uint64_t where we can use 'mdb -kwe "zfs_no_trim/W" 0'
    with your case we can corrupt kernel with 64bit update on 32bit var

    1. uint_t is just fine, especially in the context of the mdb snippet you just pasted. Note that the W format writes a uint_t-sized value:

      $ mdb
      > ::formats ! grep -w W
      W - write default radix unsigned int (4 bytes)
      
    2. These days I use /z mostly anyway :)

  3. 
      
jclulow
  1. 
      
  2. usr/src/uts/common/fs/zfs/vdev_disk.c (Diff revision 1)
     
     

    Do you think it might help with debugging if we consulted the zfs_no_trim tuneable directly, rather than having it affect the vdev_has_trim value?

    If we don't do this here, but rather consult the tuneable directly in the other places (e.g., vdev_autotrim_should_stop()) we can still see whether the TRIM bits would be active for a vdev, and we can tune it on without reimporting the pool. This might also help out when testing a potential blacklist feature?

    1. There is a lot of different ways we could go here, but this seemed best from what an end-user would see, since all of their disks will now show up as "trim unsupported" with 'zpool status -t'. I felt like it would be confusing for disks to say that trim is supported, but then we actually did not trim them. In terms of debugging, this implementation won't be a problem for now. Hopefully this is not going to stick around for very long, once I get more confidence in what needs to be fixed or improved. I won't drop this issue just now, in case more people feel like we should do something differently.

    2. I don't feel that strongly about it, and I agree that the display (which I didn't know about) would then be confusing. Feel free to drop and proceed!

    3. OK, thanks, I'll leave it as-is then.

  3. 
      
citrus
  1. Ship It!
  2. 
      
kkantor
  1. 
      
  2. 
      
jjelinek
Review request changed

Status: Closed (submitted)

Loading...