Thanks Yuri. This solution is much cleaner inasmuch as with your feature, my patch to OpenZFS code wouldn’t have to include check for every possible non rotational media type, and as well this property could potentially be useful elsewhere. If you want to get reviews on this I can rebase and update my patch to query this property instead.
James > On Mar 5, 2017, at 4:23 PM, Yuri Pankov <[email protected]> wrote: > > On Sun, 5 Mar 2017 23:29:24 +0300, Yuri Pankov wrote: >> On Sun, 5 Mar 2017 15:09:39 -0500, James Blachly wrote: >>> ZFS on Linux has had a feature "Disable LBA weighting on SSD" since August >>> 2015: >>> http://open-zfs.org/wiki/Features#Disable_LBA_Weighting_on_files_and_SSDs >>> https://github.com/zfsonlinux/zfs/commit/fb40095f5f0853946f8150481ca22602d1334dfe >>> >>> Because we have the driver property "device-solid-state" we can port this >>> over. In addition, we should add detection for virtio vioblk device, which >>> should also not use LBA weighting. >>> >>> On linux, block devices have a "non-rotational" property, so this is a one >>> liner in vdev_disk.c: >>> v->vdev_nonrot = blk_queue_nonrot(bdev_get_queue(vd->vd_bdev)); >>> >>> We do not, so the checks are a little bit more cumbersome but doable (check >>> device-solid-state != 0 and inquiry-vendor-id==Virtio separately) >>> >>> QUESTIONS >>> 1. Linux has a non rotational property for block devices. Should we? By >>> moving this out of ZFS it could be used in other places. Besides SSD, this >>> property would apply to zvols, Virtio disks, and potentially in the future, >>> network block devices (e.g. ceph) >>> 2. Is there a better or more preferred way to check for a Virtio vioblk >>> device besides checking inquiry-vendor-id ? >>> 3. Since this is a straight port of all but the code in vdev_disk.c, I did >>> not add copyright messages. This will cause git pbchk to gripe. How is this >>> typically handled? >>> 4. OpenZFS developers: your repo is out of sync with illumos-gate; I don’t >>> know how to create a PR on github that does not also include other commits >>> not yet merged into your fork. Pointers appreciated. >> >> Make changes against cloned openzfs repository, but push them as a >> branch into your illumos-gate fork, eg: >> >> git clone [email protected]:openzfs/openzfs.git >> git remote add illumos <your-fork> >> <make changes> >> git push illumos master:<new-branch> >> >> and then create PR from that branch. >> >>> --- >>> Issue: https://www.illumos.org/issues/7938 >>> >>> Webrev: http://cr.illumos.org/~webrev/jblachly/zfs_lba_weighting/ >>> >>> Git commit: >>> https://github.com/jblachly/illumos-gate/commit/863ed00ba3278bfec58a0edb504ad484fac0c10b >> >> WRT the change itself - please correct me if I'm wrong, but looks like >> all possible rotational disk are driven by sd(7D), so if the parent >> isn't sd, it's not rotational - blkdev, which is yours "Virtio", any others? >> >> For the sd itself, we could enhance the sd_check_solid_state() to set >> non-rotational device property is "Block Device Characteristics" VPD >> reports RPM == 0 (for SSDs RPM == 1) - e.g., for pvscsi disks I'm >> getting RPM == 0, same would go for iscsi, etc. > > Here's a quick attempt at doing just that: > > http://cr.illumos.org/~webrev/yurip/il-dev-rot/ > <http://cr.illumos.org/~webrev/yurip/il-dev-rot/> > > for virtual disk: > # prtconf -v /dev/dsk/c2t0d0 | grep -A1 > 'device-rotational\|device-solid-state\|inquiry-product-id' > name='device-solid-state' type=int items=1 dev=none > value=00000000 > name='device-rotational' type=int items=1 dev=none > value=00000000 > -- > name='inquiry-product-id' type=string items=1 > value='Virtual disk' > > for spinning rust: > # prtconf -v /dev/dsk/c0t5000C5004CB22B0Bd0 | grep -A1 > 'device-rotational\|device-solid-state\|inquiry-product-id' > name='device-solid-state' type=int items=1 dev=none > value=00000000 > name='device-rotational' type=int items=1 dev=none > value=00000001 > -- > name='inquiry-product-id' type=string items=1 > value='ST3450857SS' > ------------------------------------------- openzfs-developer Archives: https://www.listbox.com/member/archive/274414/=now RSS Feed: https://www.listbox.com/member/archive/rss/274414/28015062-cce53afa Modify Your Subscription: https://www.listbox.com/member/?member_id=28015062&id_secret=28015062-f966d51c Powered by Listbox: http://www.listbox.com
