Skip to contentSkip to conversation

tbio: fixes

PATCH Superseded Stanislav Kholmanskikh 2 participants 4 messages Jan 13, 2015 – Jan 15, 2015 1 message not in the archive

lore
Share & tools
lore

Stanislav Kholmanskikh

author
#1 lore raw

PATCH v2 2/2 tbio: fixes

In reply to a message not in the archive

* Wait until udev creates /dev/tbio. If udev can not create the node file in a defined timeout, create it manually.
* Clean /dev/tbio after test execution, if /dev/tbio was created manually.

Signed-off-by Stanislav Kholmanskikh <[email protected]>

---

Changes since V1:
* implemented a more active waiting
* fixed the wrong logical condition next to unlink() in cleanup()

 .../kernel/device-drivers/tbio/tbio_user/tbio.c    |   62 +++++++++-----------
 1 files changed, 28 insertions(+), 34 deletions(-)
testcases/kernel/device-drivers/tbio/tbio_user/tbio.c +28 -34
index 81085a3..fdb55b5 100644
@@ -75,12 +75,19 @@ void cleanup(void)
 	if (module_loaded)
 		tst_module_unload(NULL, module_name);
 
+	if (unlink(DEVICE_NAME) && (errno != ENOENT))
+		tst_brkm(TBROK | TERRNO, NULL, "unlink failed");
+
 	TEST_CLEANUP;
 }
 
 
 void setup(void)
 {
+	dev_t devt;
+	struct stat st;
+	unsigned int i, valid_node_created;
+
 	tst_require_root(NULL);
 
 	if (tst_kvercmp(2, 6, 0) < 0) {

@@ -91,50 +98,37 @@ void setup(void)
 	tst_module_load(cleanup, module_name, NULL);
 	module_loaded = 1;
 
-	dev_t devt;
-	struct stat st;
-
 	SAFE_FILE_SCANF(cleanup, "/sys/class/block/tbio/dev",
 		"%d:0", &TBIO_MAJOR);
 
 	devt = makedev(TBIO_MAJOR, 0);
+
 	/*
-	 * Check for the /dev/tbase node, and create if it does not
-	 * exist.
+	 * Wait until udev creates the device node.
+	 * If the node is not created or invalid, create it manually.
 	 */
-	errno = 0;
-	if (stat(DEVICE_NAME, &st)) {
-		if (errno == ENOENT) {
-			/* dev node does not exist */
-			if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR | S_IWUSR |
-				S_IRGRP | S_IWGRP, devt)) {
+	valid_node_created = 0;
+	for (i = 0; i < 5; i++) {
+		if (stat(DEVICE_NAME, &st)) {
+			if (errno != ENOENT)
 				tst_brkm(TBROK | TERRNO, cleanup,
-					"mknod failed at %s:%d",
-					__FILE__, __LINE__);
-			}
+					 "stat() failed");
 		} else {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"problem with tbase device node directory");
-		}
-	} else {
-		/*
-		 * /dev/tbio CHR device exists.  Check to make sure it is for a
-		 * block device and that it has the right major and minor.
-		 */
-		if ((!(st.st_mode & S_IFCHR)) || (st.st_rdev != devt)) {
-			/* Recreate the dev node */
-			if (!unlink(DEVICE_NAME)) {
-				if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR |
-					S_IWUSR | S_IRGRP | S_IWGRP, devt)) {
-					tst_brkm(TBROK | TERRNO, cleanup,
-						"mknod failed at %s:%d",
-						__FILE__, __LINE__);
-				}
-			} else {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"unlink failed");
+			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
+				valid_node_created = 1;
+				break;
 			}
 		}
+
+		sleep(1);
+	}
+
+	if (!valid_node_created) {
+		if (unlink(DEVICE_NAME) && (errno != ENOENT))
+			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
+		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
+			  S_IRGRP | S_IWGRP, devt))
+			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
 	}
 
 	tbio_fd = open(DEVICE_NAME, O_RDWR);
Signature · 13 lines
1.7.1


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#2 lore raw

Hi!

testcases/kernel/device-drivers/tbio/tbio_user/tbio.c void setup(void) · line 123

Quoted 7 lines
testcases/kernel/device-drivers/tbio/tbio_user/tbio.c +5 -0
+			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
+				valid_node_created = 1;
+				break;
 			}
 		}
+
+		sleep(1);

I would be inclined to check at least ten times in second. I.e. usleep(100000); here and do the for loop 50 times instead.

Each second we spend doing nothing is second not spend doing actual testing.

testcases/kernel/device-drivers/tbio/tbio_user/tbio.c void setup(void) · line 126

Quoted 3 lines
testcases/kernel/device-drivers/tbio/tbio_user/tbio.c +3 -0
+	}
+
+	if (!valid_node_created) {

Maybe we should issue TINFO message here that device wasn't created by udev and we have fallen back to manual creation.

testcases/kernel/device-drivers/tbio/tbio_user/tbio.c void setup(void) · line 132

Quoted 6 lines
testcases/kernel/device-drivers/tbio/tbio_user/tbio.c +5 -0
+		if (unlink(DEVICE_NAME) && (errno != ENOENT))
+			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
+		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
+			  S_IRGRP | S_IWGRP, devt))
+			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
 	}

Otherwise this looks good.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
author
#3 lore raw

PATCH v3 2/2 tbio: fixes

* Wait until udev creates /dev/tbio. If udev can not create the node file in a defined timeout, create it manually.
* Clean /dev/tbio after test execution, if /dev/tbio was created manually.

Signed-off-by Stanislav Kholmanskikh <[email protected]>

---
Changes since V2:

* Check for the file ten times a second.

 .../kernel/device-drivers/tbio/tbio_user/tbio.c    |   66 ++++++++++----------
 1 files changed, 32 insertions(+), 34 deletions(-)
testcases/kernel/device-drivers/tbio/tbio_user/tbio.c +32 -34
index 81085a3..b27f709 100644
@@ -75,12 +75,19 @@ void cleanup(void)
 	if (module_loaded)
 		tst_module_unload(NULL, module_name);
 
+	if (unlink(DEVICE_NAME) && (errno != ENOENT))
+		tst_brkm(TBROK | TERRNO, NULL, "unlink failed");
+
 	TEST_CLEANUP;
 }
 
 
 void setup(void)
 {
+	dev_t devt;
+	struct stat st;
+	unsigned int i, valid_node_created;
+
 	tst_require_root(NULL);
 
 	if (tst_kvercmp(2, 6, 0) < 0) {

@@ -91,50 +98,41 @@ void setup(void)
 	tst_module_load(cleanup, module_name, NULL);
 	module_loaded = 1;
 
-	dev_t devt;
-	struct stat st;
-
 	SAFE_FILE_SCANF(cleanup, "/sys/class/block/tbio/dev",
 		"%d:0", &TBIO_MAJOR);
 
 	devt = makedev(TBIO_MAJOR, 0);
+
 	/*
-	 * Check for the /dev/tbase node, and create if it does not
-	 * exist.
+	 * Wait until udev creates the device node.
+	 * If the node is not created or invalid, create it manually.
 	 */
-	errno = 0;
-	if (stat(DEVICE_NAME, &st)) {
-		if (errno == ENOENT) {
-			/* dev node does not exist */
-			if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR | S_IWUSR |
-				S_IRGRP | S_IWGRP, devt)) {
+	valid_node_created = 0;
+	for (i = 0; i < 50; i++) {
+		if (stat(DEVICE_NAME, &st)) {
+			if (errno != ENOENT)
 				tst_brkm(TBROK | TERRNO, cleanup,
-					"mknod failed at %s:%d",
-					__FILE__, __LINE__);
-			}
+					 "stat() failed");
 		} else {
-			tst_brkm(TBROK | TERRNO, cleanup,
-				"problem with tbase device node directory");
-		}
-	} else {
-		/*
-		 * /dev/tbio CHR device exists.  Check to make sure it is for a
-		 * block device and that it has the right major and minor.
-		 */
-		if ((!(st.st_mode & S_IFCHR)) || (st.st_rdev != devt)) {
-			/* Recreate the dev node */
-			if (!unlink(DEVICE_NAME)) {
-				if (mknod(DEVICE_NAME, S_IFCHR | S_IRUSR |
-					S_IWUSR | S_IRGRP | S_IWGRP, devt)) {
-					tst_brkm(TBROK | TERRNO, cleanup,
-						"mknod failed at %s:%d",
-						__FILE__, __LINE__);
-				}
-			} else {
-				tst_brkm(TBROK | TERRNO, cleanup,
-					"unlink failed");
+			if ((st.st_mode & S_IFBLK) && (st.st_rdev == devt)) {
+				valid_node_created = 1;
+				break;
 			}
 		}
+
+		usleep(100000);
+	}
+
+	if (!valid_node_created) {
+		tst_resm(TINFO,
+			 "The device file was not created by udev, "
+			 "proceeding with manual creation");
+
+		if (unlink(DEVICE_NAME) && (errno != ENOENT))
+			tst_brkm(TBROK | TERRNO, cleanup, "unlink() failed");
+		if (mknod(DEVICE_NAME, S_IFBLK | S_IRUSR | S_IWUSR |
+			  S_IRGRP | S_IWGRP, devt))
+			tst_brkm(TBROK | TERRNO, cleanup, "mknod() failed");
 	}
 
 	tbio_fd = open(DEVICE_NAME, O_RDWR);
Signature · 13 lines
1.7.1


------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list
#4 lore raw

Hi!
Acked.

Signature · 13 lines
Cyril Hrubis
[email protected]

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list