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 [email protected]
-------- "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;
}
