Skip to contentSkip to conversation

fs_fill: switch the SAFE MACROs back to ordinary syscall

PATCH New Li Wang 3 participants 9 messages Nov 10, 2017 – Nov 22, 2017 Library code

lore
Share & tools
lore

Li Wang

author
#0 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

To fix those brokens:
safe_macros.c:225: BROK: tst_fill_fs.c:43: open(mntpoint/thread48/file0,65,0700) failed: ENOSPC safe_macros.c:169: BROK: fs_fill.c:49: mkdir(mntpoint/thread21,0700) failed: ENOSPC

Signed-off-by Li Wang <[email protected]>
---
 lib/tst_fill_fs.c                     |  9 ++++++++-
 testcases/kernel/fs/fs_fill/fs_fill.c | 10 +++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)
lib/tst_fill_fs.c +8 -1
index 6b2bbbc..2775a2c 100644
@@ -40,7 +40,14 @@ void tst_fill_fs(const char *path, int verbose)
 		if (verbose)
 			tst_res(TINFO, "Creating file %s size %zu", file, len);
 
-		fd = SAFE_OPEN(file, O_WRONLY | O_CREAT, 0700);
+		fd = open(file, O_WRONLY | O_CREAT, 0700);
+		if (fd == -1) {
+			if (errno != ENOSPC)
+				tst_brk(TBROK | TERRNO, "open()");
+
+			tst_res(TINFO | TERRNO, "open()");
+			return;
+		}
 
 		while (len) {
 			ret = write(fd, buf, MIN(len, sizeof(buf)));
testcases/kernel/fs/fs_fill/fs_fill.c +9 -1
index a50a22f..f592cd5 100644
@@ -41,12 +41,20 @@ struct worker {
 
 static void *worker(void *p)
 {
+	int ret;
 	struct worker *w = p;
 	DIR *d;
 	struct dirent *ent;
 	char file[PATH_MAX];
 
-	SAFE_MKDIR(w->dir, 0700);
+	ret = mkdir(w->dir, 0700);
+	if (ret == -1) {
+		if (errno != ENOSPC)
+			tst_brk(TBROK | TERRNO, "mkdir()");
+
+		tst_res(TINFO | TERRNO, "mkdir()");
+		return NULL;
+	}
 
 	while (run) {
 		tst_fill_fs(w->dir, 0);
Signature · 1 line
2.9.3
#1 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

----- Original Message -----

Quoted 7 lines

To fix those brokens:
safe_macros.c:225: BROK: tst_fill_fs.c:43:
open(mntpoint/thread48/file0,65,0700) failed: ENOSPC safe_macros.c:169: BROK: fs_fill.c:49: mkdir(mntpoint/thread21,0700)
failed: ENOSPC

Signed-off-by Li Wang <[email protected]>

Pushed.

Thanks,
Jan

#2 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

Hi!

Quoted 8 lines
Quoted 6 lines

safe_macros.c:225: BROK: tst_fill_fs.c:43:
open(mntpoint/thread48/file0,65,0700) failed: ENOSPC safe_macros.c:169: BROK: fs_fill.c:49: mkdir(mntpoint/thread21,0700)
failed: ENOSPC

Signed-off-by Li Wang <[email protected]>

Pushed.

I do not like that the worker exits prematurely here if the mkdir fails, that's why I suggested to mkdir() all the directories before we attempt to run the the worker threads.

But I can always change that in a follow up patch...

Signature · 2 lines
Cyril Hrubis
[email protected]
author
#3 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

Quoted 6 lines · Cyril Hrubis

Hi!

Quoted 5 lines
Quoted 5 lines

safe_macros.c:225: BROK: tst_fill_fs.c:43:
open(mntpoint/thread48/file0,65,0700) failed: ENOSPC safe_macros.c:169: BROK: fs_fill.c:49: mkdir(mntpoint/thread21,0700)
failed: ENOSPC

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Li Wang <[email protected]>

Pushed.

I do not like that the worker exits prematurely here if the mkdir fails, that's why I suggested to mkdir() all the directories before we attempt to run the the worker threads.

My bad. Seems I did not fully get your point before.

Quoted 2 lines

But I can always change that in a follow up patch...

How about this following change:

testcases/kernel/fs/fs_fill/fs_fill.c +4 -10
@@ -41,21 +41,11 @@ struct worker {
 
 static void *worker(void *p)
 {
-       int ret;
        struct worker *w = p;
        DIR *d;
        struct dirent *ent;
        char file[PATH_MAX];
 
-       ret = mkdir(w->dir, 0700);
-       if (ret == -1) {
-               if (errno != ENOSPC)
-                       tst_brk(TBROK | TERRNO, "mkdir()");
-
-               tst_res(TINFO | TERRNO, "mkdir()");
-               return NULL;
-       }
-
        while (run) {
                tst_fill_fs(w->dir, 0);
 

@@ -92,6 +82,10 @@ static void testrun(void)
        for (i = 0; i < nthreads; i++) {
                snprintf(workers[i].dir, sizeof(workers[i].dir),
                         MNTPOINT "/thread%i", i + 1);
+               SAFE_MKDIR(workers[i].dir, 0700);
+       }
+
+       for (i = 0; i < nthreads; i++) {
                SAFE_PTHREAD_CREATE(&threads[i], NULL, worker, &workers[i]);
        }
Quoted 4 lines

--
Cyril Hrubis
[email protected]

Signature · 9 lines
Li Wang
[email protected]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fs_fill-create-all-the-directories-befere-run-worker.patch
Type: text/x-patch
Size: 1409 bytes
Desc: not available
URL: <http://lists.linux.it/pipermail/ltp/attachments/20171121/d78e751e/attachment.bin>
#4 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

Hi!

Quoted 29 lines
Quoted 1 line

But I can always change that in a follow up patch...

How about this following change:

testcases/kernel/fs/fs_fill/fs_fill.c +0 -10
@@ -41,21 +41,11 @@ struct worker {
 
 static void *worker(void *p)
 {
-       int ret;
        struct worker *w = p;
        DIR *d;
        struct dirent *ent;
        char file[PATH_MAX];
 
-       ret = mkdir(w->dir, 0700);
-       if (ret == -1) {
-               if (errno != ENOSPC)
-                       tst_brk(TBROK | TERRNO, "mkdir()");
-
-               tst_res(TINFO | TERRNO, "mkdir()");
-               return NULL;
-       }
-
        while (run) {
                tst_fill_fs(w->dir, 0);

testcases/kernel/fs/fs_fill/fs_fill.c static void testrun(void) · line 90

Quoted 9 lines
testcases/kernel/fs/fs_fill/fs_fill.c +4 -0
@@ -92,6 +82,10 @@ static void testrun(void)
        for (i = 0; i < nthreads; i++) {
                snprintf(workers[i].dir, sizeof(workers[i].dir),
                         MNTPOINT "/thread%i", i + 1);
+               SAFE_MKDIR(workers[i].dir, 0700);
+       }
+
+       for (i = 0; i < nthreads; i++) {
                SAFE_PTHREAD_CREATE(&threads[i], NULL, worker, &workers[i]);
        }

This one looks cleaner to me.

Signature · 2 lines
Cyril Hrubis
[email protected]
#5 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

----- Original Message -----

Quoted 29 lines

Hi!

Quoted 28 lines
Quoted 1 line

But I can always change that in a follow up patch...

How about this following change:

testcases/kernel/fs/fs_fill/fs_fill.c +0 -10
@@ -41,21 +41,11 @@ struct worker {
 
 static void *worker(void *p)
 {
-       int ret;
        struct worker *w = p;
        DIR *d;
        struct dirent *ent;
        char file[PATH_MAX];
 
-       ret = mkdir(w->dir, 0700);
-       if (ret == -1) {
-               if (errno != ENOSPC)
-                       tst_brk(TBROK | TERRNO, "mkdir()");
-
-               tst_res(TINFO | TERRNO, "mkdir()");
-               return NULL;
-       }
-
        while (run) {
                tst_fill_fs(w->dir, 0);
Quoted 13 lines

testcases/kernel/fs/fs_fill/fs_fill.c static void testrun(void) · line 91

Quoted 11 lines
testcases/kernel/fs/fs_fill/fs_fill.c +4 -0
@@ -92,6 +82,10 @@ static void testrun(void)
        for (i = 0; i < nthreads; i++) {
                snprintf(workers[i].dir, sizeof(workers[i].dir),
                         MNTPOINT "/thread%i", i + 1);
+               SAFE_MKDIR(workers[i].dir, 0700);
+       }
+
+       for (i = 0; i < nthreads; i++) {
                SAFE_PTHREAD_CREATE(&threads[i], NULL, worker,
                &workers[i]);
        }

This one looks cleaner to me.

But perhaps this should go to setup(), otherwise with "-i 2" it hits TBROK on mkdir.

I tried adding RMDIR after join, but unlink doesn't seem to remove everything:
safe_macros.c:184: BROK: fs_fill.c:108: rmdir(mntpoint/thread4) failed: ENOTEMPTY

Regards,
Jan

#6 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

----- Original Message -----

Quoted 45 lines

----- Original Message -----

Quoted 29 lines

Hi!

Quoted 28 lines
Quoted 1 line

But I can always change that in a follow up patch...

How about this following change:

testcases/kernel/fs/fs_fill/fs_fill.c +0 -10
@@ -41,21 +41,11 @@ struct worker {
 
 static void *worker(void *p)
 {
-       int ret;
        struct worker *w = p;
        DIR *d;
        struct dirent *ent;
        char file[PATH_MAX];
 
-       ret = mkdir(w->dir, 0700);
-       if (ret == -1) {
-               if (errno != ENOSPC)
-                       tst_brk(TBROK | TERRNO, "mkdir()");
-
-               tst_res(TINFO | TERRNO, "mkdir()");
-               return NULL;
-       }
-
        while (run) {
                tst_fill_fs(w->dir, 0);
Quoted 13 lines

testcases/kernel/fs/fs_fill/fs_fill.c static void testrun(void) · line 91

Quoted 11 lines
testcases/kernel/fs/fs_fill/fs_fill.c +4 -0
@@ -92,6 +82,10 @@ static void testrun(void)
        for (i = 0; i < nthreads; i++) {
                snprintf(workers[i].dir, sizeof(workers[i].dir),
                         MNTPOINT "/thread%i", i + 1);
+               SAFE_MKDIR(workers[i].dir, 0700);
+       }
+
+       for (i = 0; i < nthreads; i++) {
                SAFE_PTHREAD_CREATE(&threads[i], NULL, worker,
                &workers[i]);
        }

This one looks cleaner to me.

Quoted 7 lines

But perhaps this should go to setup(), otherwise with "-i 2" it hits TBROK on mkdir.

I tried adding RMDIR after join, but unlink doesn't seem to remove everything:
safe_macros.c:184: BROK: fs_fill.c:108: rmdir(mntpoint/thread4) failed:
ENOTEMPTY

I see, there's "break" in loop that does unlink. Which is likely by design, so we hit ENOSPC sooner.

Quoted 7 lines

Regards,
Jan

--
Mailing list info: https://lists.linux.it/listinfo/ltp

#7 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

Hi!

Quoted 10 lines
Quoted 7 lines

But perhaps this should go to setup(), otherwise with "-i 2" it hits TBROK on mkdir.

I tried adding RMDIR after join, but unlink doesn't seem to remove everything:
safe_macros.c:184: BROK: fs_fill.c:108: rmdir(mntpoint/thread4) failed:
ENOTEMPTY

I see, there's "break" in loop that does unlink. Which is likely by design, so we hit ENOSPC sooner.

That was the intention. The initial idea is that many threads race on writing into nearly full filesystem. I guess I should have written better description in the top level comment.

Signature · 2 lines
Cyril Hrubis
[email protected]
#8 lore raw

PATCH 1/1 fs_fill: switch the SAFE MACROs back to ordinary syscall

In reply to Jan Stancek

Hi!

Quoted 2 lines

But perhaps this should go to setup(), otherwise with "-i 2" it hits TBROK on mkdir.

You are right, the whole worker initialization including the MKDIR() belongs to setup() function.

Signature · 2 lines
Cyril Hrubis
[email protected]