RFC: Handle "device busy" error when registering

[Ian Pilcher <arequipeno@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

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@gmail.com
-------- "I grew up before Mark Zuckerberg invented friendship" --------
========================================================================

[-- Attachment #2: bcache-register --]
[-- Type: text/plain, Size: 65 bytes --]

#!/bin/sh
/sbin/modprobe -qba bcache
exec /usr/lib/udev/bcreg $1

[-- Attachment #3: bcreg.c --]
[-- Type: text/x-csrc, Size: 1674 bytes --]

#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;
}

[-- Attachment #4: linux-bcache-register-retval.patch --]
[-- Type: text/x-patch, Size: 1054 bytes --]

--- ./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;
 }