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

udev event storm - s390x #961

Closed
dbungert opened this issue Oct 2, 2023 · 3 comments · Fixed by #962
Closed

udev event storm - s390x #961

dbungert opened this issue Oct 2, 2023 · 3 comments · Fixed by #962
Assignees
Labels

Comments

@dbungert
Copy link

dbungert commented Oct 2, 2023

Filing as a distinct issue - storaged-project/udisks#1192 and related appear to be different problems.

I have observed a udev event storm on s390x, tested with Ubuntu Mantic (23.10), and diagnosed as follows:

a read-write open is performed

#0  0x000003fff7115bfe in __libc_open64 (file=0x2aa000d4e80 "/dev/dasda", oflag=524418) at ../sysdeps/unix/sysv/linux/open64.c:41
#1  0x000003ffe75162f0 in fdisk_assign_device () at /lib/s390x-linux-gnu/libfdisk.so.1
#2  0x000003ffe7583822 in ??? () at /lib/s390x-linux-gnu/libbd_part.so.3
#3  0x000003ffe7584cc2 in bd_part_get_disk_spec () at /lib/s390x-linux-gnu/libbd_part.so.3
#4  0x000002aa00034236 in udisks_linux_partition_table_update ()
#5  0x000002aa0002a46a in ??? ()
#6  0x000002aa0002a826 in udisks_linux_block_object_uevent ()
#7  0x000002aa0002b048 in ??? ()
#8  0x000002aa0002b086 in ??? ()
#9  0x000003fff7360bfa in ??? () at /lib/s390x-linux-gnu/libglib-2.0.so.0
#10 0x000003fff73c8376 in ??? () at /lib/s390x-linux-gnu/libglib-2.0.so.0
#11 0x000003fff736173a in g_main_loop_run () at /lib/s390x-linux-gnu/libglib-2.0.so.0
#12 0x000002aa00022376 in main ()

udisks_linux_partition_table_update() can call bd_part_get_disk_spec() which is configured to do a read-write context open in the fdisk_assign_device() call. That open triggers udev watch rules, which cause a new udev event, which may trigger the above udisks_linux_partition_table_update() again.

The scenario for that to happen would be

  1. a device with partitions
  2. but no ID_PART_TABLE_TYPE is set

https://github.com/storaged-project/udisks/blob/c7027d888c00381851d918f33a13102e7b86e188/src/udiskslinuxpartitiontable.c#L147-L157

The return value of spec doesn't seem to matter much, as the next time the loop comes around, if we still have partitions but not ID_PART_TABLE_TYPE, bd_part_get_disk_spec() will be called again.

@tbzatek
Copy link
Member

tbzatek commented Oct 2, 2023

Good catch! Seems like we should differentiate readonly scenarios for fdisk_assign_device() in case of public getter functions.

@dbungert
Copy link
Author

dbungert commented Oct 2, 2023

Good catch! Seems like we should differentiate readonly scenarios for fdisk_assign_device() in case of public getter functions.

Thanks! I do think it would be healthy for libblockdev to differentiate the read-only and read-write use cases, to reduce spurious watch events.

Short term, do you believe that disabling this code block is safe? Some info would be lost - part_type potentially for the non-ID_PART_TABLE_TYPE case if that part_type is gpt or msdos, but I think it would cut off the loop for now.
https://github.com/storaged-project/udisks/blob/c7027d888c00381851d918f33a13102e7b86e188/src/udiskslinuxpartitiontable.c#L151-L170

@tbzatek
Copy link
Member

tbzatek commented Oct 2, 2023

Short term, do you believe that disabling this code block is safe? Some info would be lost - part_type potentially for the non-ID_PART_TABLE_TYPE case if that part_type is gpt or msdos, but I think it would cut off the loop for now.

It was added recently for this UDF+VFAT corner case: storaged-project/udisks#1110. It was expected that the udev attributes are present in most cases. You can remove temporarily on your side without any significant impact. We'll fix the libblockdev side anyway.

@vojtechtrefny vojtechtrefny transferred this issue from storaged-project/udisks Oct 2, 2023
@vojtechtrefny vojtechtrefny self-assigned this Oct 2, 2023
vojtechtrefny added a commit to vojtechtrefny/libblockdev that referenced this issue Oct 3, 2023
We don't need to open the device read-write for functions like
bd_part_get_disk_spec.

Fixes: storaged-project#961
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants