scsi_id - EBUSY and serialization

PATCH Applied Harald Hoyer 2 participants 4 messages Jun 11, 2008

Share & tools

Harald Hoyer

author

2008-06-11 09:55 UTC #0 lore raw

We have a customer, who misses some symlinks for some of his scsi devices. Because of limited access and debugging facilities, I could only guess a fix.

The first patch retries opening the scsi device, if it is EBUSY. The second patch serializes access with flock.

With these patches it seems to work. I don't know if the flock patch is really necessary. Any help and comments appreciated.

Harald

From b7d94db71475a92efc11351cfd621e081b6c8bce Mon Sep 17 00:00:00 2001
From: Harald Hoyer <[email protected]>
Date: Wed, 11 Jun 2008 11:32:34 +0200
Subject: [PATCH] retry open on EBUSY

---
 extras/scsi_id/scsi_serial.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

extras/scsi_id/scsi_serial.c +8 -1

index c5cacfa..ec72c7d 100644
@@ -822,10 +822,17 @@ int scsi_get_serial (struct scsi_id_device *dev_scsi, const char *devname,
 	int fd;
 	int ind;
 	int retval;
+	int cnt = 10;
 
 	memset(dev_scsi->serial, 0, len);
 	dbg("opening %s\n", devname);
-	fd = open(devname, O_RDONLY | O_NONBLOCK);
+	while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt > 0) && errno == EBUSY) {
+		info("%s: cannot open %s: %s",
+		    dev_scsi->kernel_name, devname, strerror(errno));	  	  
+		info("retrying in 0.5 seconds");
+		usleep(500000 + (rand() % 100000) );
+		cnt--;
+	}
 	if (fd < 0) {
 		info("%s: cannot open %s: %s\n",
 		    dev_scsi->kernel, devname, strerror(errno));

Signature · 1 line

1.5.5.1

From ec3abdfd0b2e4f596c4cc988d5b7e6714cdb264e Mon Sep 17 00:00:00 2001
From: Harald Hoyer <[email protected]>
Date: Wed, 11 Jun 2008 11:42:00 +0200
Subject: [PATCH] serialize scsi_id access with flock

---
 extras/scsi_id/scsi_id.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

extras/scsi_id/scsi_id.c +17 -0

index 5eb95e8..04ae676 100644
@@ -26,6 +26,7 @@
 #include <ctype.h>
 #include <getopt.h>
 #include <sys/stat.h>
+#include <sys/file.h>
 
 #include "../../udev.h"
 #include "scsi_id.h"

@@ -601,6 +602,17 @@ static int scsi_id(char *maj_min_dev)
 	int page_code;
 	char serial_short[MAX_SERIAL_LEN] = "";
 
+ 	char flock_path[MAX_PATH_LEN];
+ 	int flock_fd = -1;
+ 
+ 	strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
+ 	strncat(flock_path, devpath, MAX_PATH_LEN-1);
+ 	flock_path[MAX_PATH_LEN-1] = 0;
+ 
+ 	flock_fd = open(flock_path, O_RDONLY);
+ 	if (flock_fd >= 0)
+ 	  flock(flock_fd, LOCK_EX);
+ 
 	set_inq_values(&dev_scsi, maj_min_dev);
 
 	/* get per device (vendor + model) options from the config file */

@@ -636,6 +648,11 @@ static int scsi_id(char *maj_min_dev)
 		retval = 0;
 	}
 
+	if (flock_fd >= 0) {
+	  flock(flock_fd, LOCK_UN);
+	  close(flock_fd);
+	}
+
 	return retval;
 }

Signature · 1 line

1.5.5.1

Kay Sievers

2008-06-11 19:26 UTC #1 lore raw

Review

Quoted 12 lines · Harald Hoyer

We have a customer, who misses some symlinks for some of his scsi devices. Because of limited access and debugging facilities, I could only guess a fix.
The first patch retries opening the scsi device, if it is EBUSY. The second patch serializes access with flock.
With these patches it seems to work. I don't know if the flock patch is really necessary. Any help and comments appreciated.
(quoted diff) +1 -1
-       fd = open(devname, O_RDONLY | O_NONBLOCK);
+       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt >

What is open() != 0 doing? I guess you want < 0?

extras/scsi_id/scsi_id.c static int scsi_id(char *maj_min_dev) · line 609

Quoted 2 lines

extras/scsi_id/scsi_id.c +2 -0

+       strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
+       strncat(flock_path, devpath, MAX_PATH_LEN-1);

Current scsi_id has no idea about sysfs anymore. I guess, we should try if the retry patch is enough.

Kay

Harald Hoyer

author

2008-06-11 19:35 UTC #2 lore raw

In reply to Harald Hoyer

Quoted 8 lines · Kay Sievers

Quoted 7 lines · Harald Hoyer
We have a customer, who misses some symlinks for some of his scsi devices. Because of limited access and debugging facilities, I could only guess a fix.
The first patch retries opening the scsi device, if it is EBUSY. The second patch serializes access with flock.

Quoted 7 lines

Review
Quoted 5 lines
With these patches it seems to work. I don't know if the flock patch is really necessary. Any help and comments appreciated.
(quoted diff) +1 -1
-       fd = open(devname, O_RDONLY | O_NONBLOCK);
+       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt >
What is open() != 0 doing? I guess you want < 0?

errr... yes :)

Quoted 8 lines

extras/scsi_id/scsi_id.c static int scsi_id(char *maj_min_dev) · line 609
Quoted 3 lines
extras/scsi_id/scsi_id.c +2 -0
+       strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
+       strncat(flock_path, devpath, MAX_PATH_LEN-1);
Current scsi_id has no idea about sysfs anymore. I guess, we should try if the retry patch is enough.
Kay

if not, we could flock s.th. like "/dev/.udev/tmp_" + major + minor

Kay Sievers

Applied

2008-06-11 21:56 UTC #3 lore raw

In reply to Harald Hoyer

Quoted 15 lines · Harald Hoyer

Quoted 14 lines · Kay Sievers
Quoted 11 lines · Harald Hoyer
We have a customer, who misses some symlinks for some of his scsi devices.
Because of limited access and debugging facilities, I could only guess a fix.
The first patch retries opening the scsi device, if it is EBUSY. The second patch serializes access with flock.
With these patches it seems to work. I don't know if the flock patch is really necessary. Any help and comments appreciated.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
(quoted diff) +1 -1
-       fd = open(devname, O_RDONLY | O_NONBLOCK);
+       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt
Quoted 1 line
What is open() != 0 doing? I guess you want < 0?
errr... yes :)

Applied.

Thanks,
Kay