On Wed, Mar 26, 2025 at 05:08:19PM -0600, Uday Shankar wrote:
On Wed, Mar 26, 2025 at 12:56:56PM -0600, Uday Shankar wrote:
On Wed, Mar 26, 2025 at 11:54:16AM -0600, Uday Shankar wrote:
ublk_abort_requests() should be called only in case of queue dying, since ublk server may open & close the char device multiple times.
Sure that is technically possible, however is any real ublk server doing this? Seems like a strange thing to do, and seems reasonable for the driver to transition the device to the nosrv state (dead or recovery, depending on flags) when the char device is closed, since in this case, no one can be handling I/O anymore.
I see ublksrv itself is doing this :(
/* Wait until ublk device is setup by udev */ static void ublksrv_check_dev(const struct ublksrv_ctrl_dev_info *info) { unsigned int max_time = 1000000, wait = 0; char buf[64];
snprintf(buf, 64, "%s%d", "/dev/ublkc", info->dev_id);
while (wait < max_time) { int fd = open(buf, O_RDWR);
if (fd > 0) { close(fd); break; } usleep(100000); wait += 100000;} }
This seems related to some failures in ublksrv tests
Actually this is the only issue I'm seeing - after patching this up in ublksrv, make T=generic test appears to pass - I don't see any logs indicating failures, and no kernel panics.
Yes, that is one example.
Your patch breaks any application which opens ublk char more than one times. And it is usually one common practice to allow device to be opened multiple times.
Maybe one utility opens the char device unexpected, systemd usually open & read block device, not sure if it opens char device.
I try to add change against your patch to abort requests only in ->release() when queue becomes dying, and the added check triggers kernel panic.
So the question is, does this patch break existing ublk servers? It does
It should break any application which depends on libublksrv
break ublksrv as shown above, but I think one could argue that the above code is just testing for file existence, and it's a bit weird to do that by opening and closing the file (especially given that it's a device special file). It can be patched to just use access or something instead.
Even though libublksrv is the only one which has such usage, it is impossible to force the user to upgrade the library, but user still may upgrade to the latest kernel...
Thanks, Ming