Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow polling single-device md devices #488

Merged
merged 2 commits into from
Dec 28, 2021

Conversation

HippoBaro
Copy link
Member

Linux is weird. LVM devices can present themselves as md devices with a
single underlying device. That device may be polled (but ones with
more than a single underlying device can't). Right now, if a
device is md, we opt out of polling entirely, even though polling is
allowed under that edge-case.

This commit changes the logic to disable polling only if an md device
has more than a single device underneath it.

Note: I have not tested this under a RAID configuration with a single
disk. Logic would have that it would work too, but as we established,
Linux is weird.

Linux is weird. LVM devices can present themselves as md devices with a
single underlying device. That device may be polled (but ones with
more than a single underlying device can't). Right now, if a
device is md, we opt out of polling entirely, even though polling is
allowed under that edge-case.

This commit changes the logic to disable polling only if an md device
has more than a single device underneath it.
@HippoBaro HippoBaro merged commit 16d9166 into DataDog:master Dec 28, 2021
@HippoBaro HippoBaro deleted the fix_polling_on_single_md_device branch December 28, 2021 15:04
@youjiali1995
Copy link
Contributor

I got this error with this patch

$ cargo run --release --example storage -- --dir tmp --size-gb 10
    Finished release [optimized] target(s) in 0.07s
     Running `target/release/examples/storage --dir tmp --size-gb 10`
Buffered I/O: Wrote 10.74 GB in 12.582746071s, 853.34 MB/s
Buffered I/O: Closed in 38.415498ms, Amortized total 850.75 MB/s
thread 'unnamed-1' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Operation not supported (os error 95), op: Writing path: Some(\"/home/youjiali1995/Program/hackathon/2021/glommio/tmp/benchfiles/benchfile-dio-1\") with fd: Some(8)" }', examples/storage.rs:55:43
$ lsblk
NAME            MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINT
loop0             7:0    0  99.4M  1 loop  /snap/core/11993
loop1             7:1    0  99.5M  1 loop  /snap/core/11798
loop2             7:2    0  55.5M  1 loop  /snap/core18/2253
loop3             7:3    0  55.5M  1 loop  /snap/core18/2246
loop4             7:4    0 995.2M  1 loop  /snap/intellij-idea-ultimate/338
loop5             7:5    0 987.3M  1 loop  /snap/intellij-idea-ultimate/333
nvme0n1         259:0    0 953.9G  0 disk
├─nvme0n1p1     259:1    0   498M  0 part  /boot/efi
├─nvme0n1p2     259:2    0     4G  0 part  /recovery
├─nvme0n1p3     259:3    0 945.4G  0 part
│ └─cryptdata   253:0    0 945.4G  0 crypt
│   └─data-root 253:1    0 945.4G  0 lvm   /
└─nvme0n1p4     259:4    0     4G  0 part
  └─cryptswap   253:2    0     4G  0 crypt
$ ll /sys/dev/block/ | grep '253:0'
lrwxrwxrwx 0 root 31 12月  2021 253:0 -> ../../devices/virtual/block/dm-0
$ ls /sys/devices/virtual/block/dm-1/slaves
dm-0

@HippoBaro
Copy link
Member Author

Thanks for reporting @youjiali1995!

Can you confirm that "/home/youjiali1995/Program/hackathon/2021/glommio/tmp/benchfiles/benchfile-dio-1" is hosted on a nvme device formatted with ext4 or xfs?

Did you try to drop the commit from this PR? Did it work afterward?

@youjiali1995
Copy link
Contributor

Can you confirm that "/home/youjiali1995/Program/hackathon/2021/glommio/tmp/benchfiles/benchfile-dio-1" is hosted on a nvme device formatted with ext4 or xfs?

Yes.

NAME            FSTYPE      LABEL UUID                                   FSAVAIL FSUSE% MOUNTPOINT
nvme0n1
├─nvme0n1p1     vfat              8E49-FC19                               135.1M    73% /boot/efi
├─nvme0n1p2     vfat              8E49-FBC6                                 1.4G    66% /recovery
├─nvme0n1p3     crypto_LUKS       430cb290-a039-49cd-aa54-9c536a7218cf
│ └─cryptdata   LVM2_member       60FMRG-mLf2-g5KE-CC08-jecP-5cCN-Pqk5GR
│   └─data-root ext4              c191d23f-cab1-4661-812d-7f0c8f1cbcfa      250G    68% /

Did you try to drop the commit from this PR? Did it work afterward?

Yes.

@HippoBaro
Copy link
Member Author

Hum, this is curious. Here's my computer where this patch works as expected.

$ lsblk -fs -M
    NAME                           FSTYPE      FSVER    LABEL   UUID                                   FSAVAIL FSUSE% MOUNTPOINTS
    zram0                                                                                                             [SWAP]
┌┈▶ fedora_localhost--live-root    ext4        1.0              227f268a-32ed-4881-b4e6-cd06e9b12bed     38.3G    39% /
├┈▶ fedora_localhost--live-swap    swap        1                1b6cbced-82be-470f-bcab-02b293d06b81                  [SWAP]
├┈▶ fedora_localhost--live-home    ext4        1.0              91e57d30-a476-43e9-a6e9-d1a6b3503727    154.7G    42% /home
└┬▶ fedora_localhost--live-vol_xfs xfs                  vol_xfs 9034f8df-85e3-42ee-a0a1-a61f22d1c572     73.2G     1% /var/lib/nvme
 └┈┈nvme0n1p3                      LVM2_member LVM2 001         gZNa7p-R9pj-ZAY1-1Pox-1gKK-W9Py-vZ9LWc
┌┈▶ nvme0n1p1                      vfat        FAT32            AE59-94DF                                 585M     2% /boot/efi
└┬▶ nvme0n1p2                      ext4        1.0              e8434dd4-adef-4fd6-92e6-0e4f38c3298a    359.3M    56% /boot
 └┈┈nvme0n1

The storage example works in both /home (ext4) and /var/lib/nvme (xfs). Like you, these partitions are managed using LVM2. The only difference is that your LVM instance is itself hosted under a dm-crypt device. So I wonder if that's playing a role here.

@glommer
Copy link
Collaborator

glommer commented Jan 6, 2022

@HippoBaro we should revert this , and keep investigating in parallel.

HippoBaro added a commit to HippoBaro/glommio that referenced this pull request Jan 6, 2022
HippoBaro added a commit that referenced this pull request Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants