On 07/18/2014 07:11 PM, Ian Pilcher wrote: > Suggested fix: > > (1) Change bcache_register to return -EBUSY in the device busy case > (while still returning -EINVAL in the already registered case). > > (2) Change bcache-register to check the exit code of the registration > attempt and retry in the EBUSY case. > > Does this make sense? I've gone ahead and implemented an initial version of this approach. See the attached files: * linux-bcache-register-retval.patch - Makes bcache_register return -EBUSY when it is unable to get exclusive access to the device, but the device is not already registered. It still returns -EINVAL in all other error cases. This allows userspace to distinguish the "device busy" case from other errors. I couldn't find an easy way to make this determination from a shell script, though, so I created ... * bcreg.c - This does most of the work that was previously done in the bcache-register script. Specifically, it will wait for the sysfs register file to be created and then write the name of the device to that file -- retrying if it encounters an EBUSY. * bcache-register - bcreg doesn't call modprobe, so this script is still required. It now calls bcreg to register the device. Thoughts? -- ======================================================================== Ian Pilcher arequipeno@xxxxxxxxx -------- "I grew up before Mark Zuckerberg invented friendship" -------- ========================================================================
#!/bin/sh /sbin/modprobe -qba bcache exec /usr/lib/udev/bcreg $1
#include <stdlib.h> #include <string.h> #include <unistd.h> #include <stdio.h> #include <fcntl.h> #include <errno.h> #include <time.h> /* How often to wake up and check when waiting for something; 1/10 second */ #define WAKEUP_NSEC 100000000L /* Timeouts are in multiples of WAKEUP_NSEC */ #define SYSFS_TIMEOUT 50 /* 5 seconds */ #define DEVICE_TIMEOUT 50 /* 5 seconds */ static const char sysfs_reg[] = "/sys/fs/bcache/register"; int main(int argc, char *argv[]) { struct timespec timeout; size_t len; int i, fd; if (argc != 2) { fprintf(stderr, "USAGE: %s <BLOCK_DEVICE>\n", argv[0]); exit(EXIT_FAILURE); } i = SYSFS_TIMEOUT; while (1) { fd = open(sysfs_reg, O_WRONLY); if (fd != -1) break; if (errno != ENOENT) { perror(sysfs_reg); exit(EXIT_FAILURE); } if (i-- == 0) { fprintf(stderr, "Timed out waiting for %s\n", sysfs_reg); exit(EXIT_FAILURE); } timeout.tv_sec = 0; timeout.tv_nsec = WAKEUP_NSEC; if (nanosleep(&timeout, NULL) == -1) { perror("nanosleep"); exit(EXIT_FAILURE); } } len = strlen(argv[1]); i = DEVICE_TIMEOUT; while (1) { if (lseek(fd, 0, SEEK_SET) == -1) { perror(sysfs_reg); close(fd); exit(EXIT_FAILURE); } if (write(fd, argv[1], len) == (ssize_t)len) break; if (errno != EBUSY) { perror(argv[1]); close(fd); exit(EXIT_FAILURE); } if (i-- == 0) { fprintf(stderr, "Timed out waiting for %s\n", argv[1]); close(fd); exit(EXIT_FAILURE); } timeout.tv_sec = 0; timeout.tv_nsec = WAKEUP_NSEC; if (nanosleep(&timeout, NULL) == -1) { perror("nanosleep"); close(fd); exit(EXIT_FAILURE); } } close(fd); return 0; }
--- ./drivers/md/bcache/super.c.orig 2014-07-21 09:43:03.599875510 -0400 +++ ./drivers/md/bcache/super.c 2014-07-21 11:22:56.774478317 -0400 @@ -1924,7 +1924,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr, const char *buffer, size_t size) { - ssize_t ret = size; + ssize_t ret = -EINVAL; const char *err = "cannot allocate memory"; char *path = NULL; struct cache_sb *sb = NULL; @@ -1945,10 +1945,12 @@ if (IS_ERR(bdev)) { if (bdev == ERR_PTR(-EBUSY)) { bdev = lookup_bdev(strim(path)); - if (!IS_ERR(bdev) && bch_is_open(bdev)) + if (!IS_ERR(bdev) && bch_is_open(bdev)) { err = "device already registered"; - else + } else { err = "device busy"; + ret = -EBUSY; + } } goto err; } @@ -1976,6 +1978,9 @@ register_cache(sb, sb_page, bdev, ca); } + + ret = size; + out: if (sb_page) put_page(sb_page); @@ -1989,7 +1994,6 @@ err: if (attr != &ksysfs_register_quiet) pr_info("error opening %s: %s", path, err); - ret = -EINVAL; goto out; }