Add read_all file systems test
PATCH v2 1/1 Add read_all file systems test
While using a shell script I wrote, which dumps the contents of /sys and /proc (submitted in a separate patch), I found some minor kernel bugs. There is already a test specifically for /proc however it is attempting to verify the behavior of particular files. This test is more general and can be applied to any file system.
---
V2: Use a queue structure instead of pipes for distributing work as well as
many other small changes.
runtest/fs | 4 + testcases/kernel/fs/read_all/.gitignore | 1 + testcases/kernel/fs/read_all/Makefile | 23 ++ testcases/kernel/fs/read_all/read_all.c | 395 ++++++++++++++++++++++++++++++++ 4 files changed, 423 insertions(+) create mode 100644 testcases/kernel/fs/read_all/.gitignore create mode 100644 testcases/kernel/fs/read_all/Makefile create mode 100644 testcases/kernel/fs/read_all/read_all.c
runtest/fs +4 -0
@@ -69,6 +69,10 @@ fs_di fs_di -d $TMPDIR # Was not sure why it should reside in runtest/crashme and won´t get tested ever proc01 proc01 -m 128 +read_all_dev read_all -d /dev -q -r 10 +read_all_proc read_all -d /proc -q -r 10 +read_all_sys read_all -d /sys -q -r 10 + #Run the File System Race Condition Check tests as well fs_racer fs_racer.sh -t 5
testcases/kernel/fs/read_all/.gitignore new file +1 -0
@@ -0,0 +1 @@ +read_all
testcases/kernel/fs/read_all/Makefile new file +23 -0
@@ -0,0 +1,23 @@ +# Copyright (c) 2017 Linux Test Project +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License as +# published by the Free Software Foundation; either version 2 of +# the License, or (at your option) any later version. +# +# This program is distributed in the hope that it would be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +top_srcdir ?= ../../../.. + +include $(top_srcdir)/include/mk/testcases.mk + +CFLAGS += -D_GNU_SOURCE +LDFLAGS += -lpthread + +include $(top_srcdir)/include/mk/generic_leaf_target.mk
testcases/kernel/fs/read_all/read_all.c new file +395 -0
395 lines not on this page · raw message
Signature · 1 line
2.15.1
PATCH v2 1/1 Add read_all file systems test
Hi!
Quoted 16 lines
testcases/kernel/fs/read_all/read_all.c new file +16
-0+#define QUEUE_SIZE 16384 +#define BUFFER_SIZE 1024 +#define MAX_PATH 4096 +#define MAX_DISPLAY 40 + +struct queue { +sem_t sem; +int front; +int back; +char data[QUEUE_SIZE ]; +}; + +struct worker { +pid_t pid; +struct queue *q; +};
testcases/kernel/fs/read_all/read_all.c line 77
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +enum dent_action { +DA_UNKNOWN , +DA_IGNORE , +DA_READ , +DA_VISIT , +}; + +static char *verbose; +static char *quite;
^ quiet?
Quoted 8 lines
testcases/kernel/fs/read_all/read_all.c new file +8
-0+static char *root_dir; +static char *exclude; +static char *str_repeat; +static int repeat =1 ; +static long worker_count; +static struct worker *workers; + +static struct tst_option options[] = {
testcases/kernel/fs/read_all/read_all.c line 95
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ {"v" , &verbose, + "-v Print information about successful reads"}, + {"q", &quite, + "-q Don't print file read or open errors"}, + {"d:", &root_dir, + "-d path Path to the directory to read from, defaults to /sys"}, + {"e:", &exclude, + "-e pattern Ignore files which match an 'extended' pattern, see fnmatch(3)"}, + {"r:", &str_repeat, + "-r count The number of times to schedule a file for reading"},
I'm not sure repeat is the right name for parameter, I do not have much better idea though, maybe iteration.
Quoted 96 lines
testcases/kernel/fs/read_all/read_all.c new file +96
-0+ {NULL, NULL, NULL} +}; + +static int queue_pop (struct queue *q,char *buf) +{ +int i = q->front, j =0 ; + +sem_wait (&q->sem); + +if (!q->data[i]) +return 0 ; + +while (q->data[i]) { + buf[j] = q->data[i]; + +if (++j >=BUFFER_SIZE -1 ) +tst_brk (TBROK ,"Buffer is too small for path" ); +if (++i >=QUEUE_SIZE ) + i =0 ; + } + + buf[j] ='\0' ; +tst_atomic_store (i +1 , &q->front); + +return 1 ; +} + +static int queue_push (struct queue *q,const char *buf) +{ +int i = q->back, j =0 ; +int front =tst_atomic_load (&q->front); + +while (buf[j]) { + q->data[i] = buf[j]; + + ++j; +if (++i >=QUEUE_SIZE ) + i =0 ; +if (i == front) +return 0 ; + } + q->data[i] ='\0' ; + + q->back = i +1 ; +sem_post (&q->sem); + +return 1 ; +} + +static struct queue *queue_init (void ) +{ +struct queue *q =SAFE_MMAP (NULL ,sizeof (*q), +PROT_READ |PROT_WRITE , +MAP_SHARED |MAP_ANONYMOUS , +0 ,0 ); + +sem_init (&q->sem,1 ,0 ); + q->front =0 ; + q->back =0 ; + +return q; +} + +static void queue_destroy (struct queue *q,int is_worker) +{ +if (is_worker) +sem_destroy (&q->sem); + +SAFE_MUNMAP (q,sizeof (*q)); +} + +static void sanitize_str (char *buf,ssize_t count) +{ +int i; + +for (i =0 ; i <MIN (count,MAX_DISPLAY ); i++) +if (!isprint (buf[i])) + buf[i] =' ' ; + +if (count <=MAX_DISPLAY ) + buf[count] ='\0' ; +else +strcpy (buf +MAX_DISPLAY ,"..." ); +} + +static void read_test (const char *path) +{ +char buf[BUFFER_SIZE ]; +int fd; +ssize_t count; + +if (exclude && !fnmatch (exclude, path,FNM_EXTMATCH )) { +if (verbose) +tst_res (TINFO ,"Ignoring %s" , path); +return ; + }
testcases/kernel/fs/read_all/read_all.c line 201
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +if (verbose) +tst_res (TINFO ,"%s(%s)" , __func__, path); + + fd =open (path,O_RDONLY |O_NONBLOCK ); +if (fd <0 && !quite) { +tst_res (TINFO |TERRNO ,"open(%s)" , path); +return ; + }else if (fd <0 ) +return ;
I guess that this would be a bit more readable:
if (fd < 0) {
if (!quiet)
tst_res(...)
return;
}Quoted 18 lines
testcases/kernel/fs/read_all/read_all.c new file +18
-0+ count =read (fd, buf,sizeof (buf) -1 ); +if (count >0 && verbose) { +sanitize_str (buf, count); +tst_res (TINFO ,"read(%s, buf) = %ld, buf = %s" , + path, count, buf); + }else if (!count && verbose) +tst_res (TINFO ,"read(%s) = EOF" , path); + +else if (count <0 && !quite) +tst_res (TINFO |TERRNO ,"read(%s)" , path); + +SAFE_CLOSE (fd); +} + +static int worker_run (struct worker *self) +{ +int ret; +char buf[BUFFER_SIZE ];
testcases/kernel/fs/read_all/read_all.c
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+struct sigaction term_sa = { + .sa_handler =SIG_IGN , + .sa_flags =0 , + }; +struct queue *q = self->q; + +sigaction (SIGTTIN , &term_sa,NULL ); + +for (ret =queue_pop (q, buf); ret; ret =queue_pop (q, buf)) +read_test (buf);
As much as I do like to strech for loop for everything this would be probably more elegant as an infinite loop with a break.
for (;;) {
if (!queue_pop(q, buf))
break;
read_test(buf);
}testcases/kernel/fs/read_all/read_all.c line 232
Quoted 2 lines
testcases/kernel/fs/read_all/read_all.c new file +2
-0+queue_destroy (q,1 ); +fflush (stdout);
What are trying to flush here? The tst_res() messages are printed into the stderr btw.
Quoted 25 lines
testcases/kernel/fs/read_all/read_all.c new file +25
-0+return 0 ; +} + +static void spawn_workers (void ) +{ +int i; +struct worker *wa = workers; + +bzero (workers, worker_count *sizeof (*workers)); + +for (i =0 ; i < worker_count; i++) { + wa[i].q =queue_init (); + wa[i].pid =SAFE_FORK (); +if (!wa[i].pid) +exit (worker_run (wa + i)); + } +} + +static void stop_workers (void ) +{ +const char stop_code[1 ] = {'\0' }; +int i; + +if (!workers) +return ;
testcases/kernel/fs/read_all/read_all.c line 267
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +for (i =0 ; i < worker_count; i++) +if (workers[i].q) +queue_push (workers[i].q, stop_code); + +for (i =0 ; i < worker_count; i++) +if (workers[i].q) { +queue_destroy (workers[i].q,0 ); + workers[i].q =0 ; + }
FYI LKML coding style preferes curly braces around the block in a case that it spans over multiple lines.
Quoted 31 lines
testcases/kernel/fs/read_all/read_all.c new file +31
-0+} + +static void sched_work (const char *path) +{ +static int cur; +int push_attempts =0 , pushed; + +while (1 ) { + pushed =queue_push (workers[cur].q, path); + +if (++cur >= worker_count) + cur =0 ; + +if (pushed) +break ; + +if (++push_attempts > worker_count) { +usleep (100 ); + push_attempts =0 ; + } + } +} + +static void rep_sched_work (const char *path,int rep) +{ +int i; + +for (i =0 ; i < rep; i++) +sched_work (path); +} +
testcases/kernel/fs/read_all/read_all.c line 308
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +9
-0+static void setup (void ) +{ +if (tst_parse_int (str_repeat, &repeat,1 ,INT_MAX )) +tst_brk (TBROK , +"Invalid repeat (-r) argument: '%s'" , str_repeat); +if (!root_dir) +tst_brk (TBROK ,"The directory argument (-d) is required" ); + + worker_count =MIN (MAX (SAFE_SYSCONF (_SC_NPROCESSORS_ONLN) -1 ,1 ),15 );
We do have tst_ncpus() because there are older distros that do not define the _SC_* macros, please use that one instead here.
Also we cap the number of workers on 15 here, shouldn't that be at least macro constant? I also wonder if we should supply command line option to override the number of workers.
testcases/kernel/fs/read_all/read_all.c line 314
Quoted 6 lines
testcases/kernel/fs/read_all/read_all.c new file +6
-0+ workers =SAFE_MALLOC (worker_count *sizeof (*workers)); +} + +static void cleanup (void ) +{ +stop_workers ();
We are stopping the workers at the end of the run(), do we really need to stop them here as well?
Quoted 70 lines
testcases/kernel/fs/read_all/read_all.c new file +70
-0+free (workers); +} + +static void visit_dir (const char *path) +{ +DIR *dir; +struct dirent *dent; +struct stat dent_st; +char dent_path[MAX_PATH ]; +enum dent_action act; + + dir =opendir (path); +if (!dir) { +tst_res (TINFO |TERRNO ,"opendir(%s)" , path); +return ; + } + +while (1 ) { + errno =0 ; + dent =readdir (dir); +if (!dent && errno) { +tst_res (TINFO |TERRNO ,"readdir(%s)" , path); +break ; + }else if (!dent) +break ; + +if (!strcmp (dent->d_name,"." ) || + !strcmp (dent->d_name,".." )) +continue ; + +if (dent->d_type ==DT_DIR ) + act =DA_VISIT ; +else if (dent->d_type ==DT_LNK ) + act =DA_IGNORE ; +else if (dent->d_type ==DT_UNKNOWN ) + act =DA_UNKNOWN ; +else + act =DA_READ ; + +snprintf (dent_path,MAX_PATH , +"%s/%s" , path, dent->d_name); + +if (act ==DA_UNKNOWN ) { +if (lstat (dent_path, &dent_st)) +tst_res (TINFO |TERRNO ,"lstat(%s)" , path); +else if ((dent_st.st_mode &S_IFMT ) ==S_IFDIR ) + act =DA_VISIT ; +else if ((dent_st.st_mode &S_IFMT ) ==S_IFLNK ) + act =DA_IGNORE ; +else + act =DA_READ ; + } + +if (act ==DA_VISIT ) +visit_dir (dent_path); +else if (act ==DA_READ ) +rep_sched_work (dent_path, repeat); + } + +if (closedir (dir)) +tst_res (TINFO |TERRNO ,"closedir(%s)" , path); +} + +static void run (void ) +{ +spawn_workers (); +visit_dir (root_dir); +stop_workers (); + +tst_reap_children ();
testcases/kernel/fs/read_all/read_all.c line 394
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+tst_res (TPASS ,"Finished reading files" ); +} + +static struct tst_test test = { + .options = options, + .setup = setup, + .cleanup = cleanup, + .test_all = run, + .forks_child =1 , +};
The rest looks good to me.
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v2 1/1 Add read_all file systems test
Hello,
Quoted 207 lines · Cyril Hrubis
Hi!
Quoted 16 lines
testcases/kernel/fs/read_all/read_all.c new file +16
-0+#define QUEUE_SIZE 16384 +#define BUFFER_SIZE 1024 +#define MAX_PATH 4096 +#define MAX_DISPLAY 40 + +struct queue { +sem_t sem; +int front; +int back; +char data[QUEUE_SIZE ]; +}; + +struct worker { +pid_t pid; +struct queue *q; +};testcases/kernel/fs/read_all/read_all.c line 77
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +enum dent_action { +DA_UNKNOWN , +DA_IGNORE , +DA_READ , +DA_VISIT , +}; + +static char *verbose; +static char *quite;^ quiet?Quoted 8 lines
testcases/kernel/fs/read_all/read_all.c new file +8
-0+static char *root_dir; +static char *exclude; +static char *str_repeat; +static int repeat =1 ; +static long worker_count; +static struct worker *workers; + +static struct tst_option options[] = {testcases/kernel/fs/read_all/read_all.c line 95
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ {"v" , &verbose, + "-v Print information about successful reads"}, + {"q", &quite, + "-q Don't print file read or open errors"}, + {"d:", &root_dir, + "-d path Path to the directory to read from, defaults to /sys"}, + {"e:", &exclude, + "-e pattern Ignore files which match an 'extended' pattern, see fnmatch(3)"}, + {"r:", &str_repeat, + "-r count The number of times to schedule a file for reading"},I'm not sure repeat is the right name for parameter, I do not have much better idea though, maybe iteration.
Quoted 96 lines
testcases/kernel/fs/read_all/read_all.c new file +96
-0+ {NULL, NULL, NULL} +}; + +static int queue_pop (struct queue *q,char *buf) +{ +int i = q->front, j =0 ; + +sem_wait (&q->sem); + +if (!q->data[i]) +return 0 ; + +while (q->data[i]) { + buf[j] = q->data[i]; + +if (++j >=BUFFER_SIZE -1 ) +tst_brk (TBROK ,"Buffer is too small for path" ); +if (++i >=QUEUE_SIZE ) + i =0 ; + } + + buf[j] ='\0' ; +tst_atomic_store (i +1 , &q->front); + +return 1 ; +} + +static int queue_push (struct queue *q,const char *buf) +{ +int i = q->back, j =0 ; +int front =tst_atomic_load (&q->front); + +while (buf[j]) { + q->data[i] = buf[j]; + + ++j; +if (++i >=QUEUE_SIZE ) + i =0 ; +if (i == front) +return 0 ; + } + q->data[i] ='\0' ; + + q->back = i +1 ; +sem_post (&q->sem); + +return 1 ; +} + +static struct queue *queue_init (void ) +{ +struct queue *q =SAFE_MMAP (NULL ,sizeof (*q), +PROT_READ |PROT_WRITE , +MAP_SHARED |MAP_ANONYMOUS , +0 ,0 ); + +sem_init (&q->sem,1 ,0 ); + q->front =0 ; + q->back =0 ; + +return q; +} + +static void queue_destroy (struct queue *q,int is_worker) +{ +if (is_worker) +sem_destroy (&q->sem); + +SAFE_MUNMAP (q,sizeof (*q)); +} + +static void sanitize_str (char *buf,ssize_t count) +{ +int i; + +for (i =0 ; i <MIN (count,MAX_DISPLAY ); i++) +if (!isprint (buf[i])) + buf[i] =' ' ; + +if (count <=MAX_DISPLAY ) + buf[count] ='\0' ; +else +strcpy (buf +MAX_DISPLAY ,"..." ); +} + +static void read_test (const char *path) +{ +char buf[BUFFER_SIZE ]; +int fd; +ssize_t count; + +if (exclude && !fnmatch (exclude, path,FNM_EXTMATCH )) { +if (verbose) +tst_res (TINFO ,"Ignoring %s" , path); +return ; + }testcases/kernel/fs/read_all/read_all.c line 201
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +if (verbose) +tst_res (TINFO ,"%s(%s)" , __func__, path); + + fd =open (path,O_RDONLY |O_NONBLOCK ); +if (fd <0 && !quite) { +tst_res (TINFO |TERRNO ,"open(%s)" , path); +return ; + }else if (fd <0 ) +return ;I guess that this would be a bit more readable:
if (fd < 0) { if (!quiet) tst_res(...) return; }Quoted 18 lines
testcases/kernel/fs/read_all/read_all.c new file +18
-0+ count =read (fd, buf,sizeof (buf) -1 ); +if (count >0 && verbose) { +sanitize_str (buf, count); +tst_res (TINFO ,"read(%s, buf) = %ld, buf = %s" , + path, count, buf); + }else if (!count && verbose) +tst_res (TINFO ,"read(%s) = EOF" , path); + +else if (count <0 && !quite) +tst_res (TINFO |TERRNO ,"read(%s)" , path); + +SAFE_CLOSE (fd); +} + +static int worker_run (struct worker *self) +{ +int ret; +char buf[BUFFER_SIZE ];testcases/kernel/fs/read_all/read_all.c
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+struct sigaction term_sa = { + .sa_handler =SIG_IGN , + .sa_flags =0 , + }; +struct queue *q = self->q; + +sigaction (SIGTTIN , &term_sa,NULL ); + +for (ret =queue_pop (q, buf); ret; ret =queue_pop (q, buf)) +read_test (buf);As much as I do like to strech for loop for everything this would be probably more elegant as an infinite loop with a break.
for (;;) { if (!queue_pop(q, buf)) break; read_test(buf); }
Quoted 5 lines
testcases/kernel/fs/read_all/read_all.c line 232
Quoted 2 lines
testcases/kernel/fs/read_all/read_all.c new file +2
-0+queue_destroy (q,1 ); +fflush (stdout);What are trying to flush here? The
tst_res()messages are printed into the stderr btw.
I found that the pass message was being written after some of the childrens' information messages. Calling fflush on stdout prevents that from happening even though it is the wrong fd...
I have changed it to stderr, which seems more correct.
Quoted 89 lines
Quoted 26 lines
testcases/kernel/fs/read_all/read_all.c new file +25
-0+return 0 ; +} + +static void spawn_workers (void ) +{ +int i; +struct worker *wa = workers; + +bzero (workers, worker_count *sizeof (*workers)); + +for (i =0 ; i < worker_count; i++) { + wa[i].q =queue_init (); + wa[i].pid =SAFE_FORK (); +if (!wa[i].pid) +exit (worker_run (wa + i)); + } +} + +static void stop_workers (void ) +{ +const char stop_code[1 ] = {'\0' }; +int i; + +if (!workers) +return ;testcases/kernel/fs/read_all/read_all.c line 267
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+ +for (i =0 ; i < worker_count; i++) +if (workers[i].q) +queue_push (workers[i].q, stop_code); + +for (i =0 ; i < worker_count; i++) +if (workers[i].q) { +queue_destroy (workers[i].q,0 ); + workers[i].q =0 ; + }FYI LKML coding style preferes curly braces around the block in a case that it spans over multiple lines.
Quoted 31 lines
testcases/kernel/fs/read_all/read_all.c new file +31
-0+} + +static void sched_work (const char *path) +{ +static int cur; +int push_attempts =0 , pushed; + +while (1 ) { + pushed =queue_push (workers[cur].q, path); + +if (++cur >= worker_count) + cur =0 ; + +if (pushed) +break ; + +if (++push_attempts > worker_count) { +usleep (100 ); + push_attempts =0 ; + } + } +} + +static void rep_sched_work (const char *path,int rep) +{ +int i; + +for (i =0 ; i < rep; i++) +sched_work (path); +} +testcases/kernel/fs/read_all/read_all.c line 308
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +9
-0+static void setup (void ) +{ +if (tst_parse_int (str_repeat, &repeat,1 ,INT_MAX )) +tst_brk (TBROK , +"Invalid repeat (-r) argument: '%s'" , str_repeat); +if (!root_dir) +tst_brk (TBROK ,"The directory argument (-d) is required" ); + + worker_count =MIN (MAX (SAFE_SYSCONF (_SC_NPROCESSORS_ONLN) -1 ,1 ),15 );We do have
tst_ncpus()because there are older distros that do not define the _SC_* macros, please use that one instead here.Also we cap the number of workers on 15 here, shouldn't that be at least macro constant? I also wonder if we should supply command line option to override the number of workers.
Quoted 9 lines
testcases/kernel/fs/read_all/read_all.c line 314
Quoted 6 lines
testcases/kernel/fs/read_all/read_all.c new file +6
-0+ workers =SAFE_MALLOC (worker_count *sizeof (*workers)); +} + +static void cleanup (void ) +{ +stop_workers ();We are stopping the workers at the end of the
run(), do we really need to stop them here as well?
spawn_workers calls SAFE_FORK which could fail after a few workers have already been spawned.
Quoted 83 lines
Quoted 71 lines
testcases/kernel/fs/read_all/read_all.c new file +70
-0+free (workers); +} + +static void visit_dir (const char *path) +{ +DIR *dir; +struct dirent *dent; +struct stat dent_st; +char dent_path[MAX_PATH ]; +enum dent_action act; + + dir =opendir (path); +if (!dir) { +tst_res (TINFO |TERRNO ,"opendir(%s)" , path); +return ; + } + +while (1 ) { + errno =0 ; + dent =readdir (dir); +if (!dent && errno) { +tst_res (TINFO |TERRNO ,"readdir(%s)" , path); +break ; + }else if (!dent) +break ; + +if (!strcmp (dent->d_name,"." ) || + !strcmp (dent->d_name,".." )) +continue ; + +if (dent->d_type ==DT_DIR ) + act =DA_VISIT ; +else if (dent->d_type ==DT_LNK ) + act =DA_IGNORE ; +else if (dent->d_type ==DT_UNKNOWN ) + act =DA_UNKNOWN ; +else + act =DA_READ ; + +snprintf (dent_path,MAX_PATH , +"%s/%s" , path, dent->d_name); + +if (act ==DA_UNKNOWN ) { +if (lstat (dent_path, &dent_st)) +tst_res (TINFO |TERRNO ,"lstat(%s)" , path); +else if ((dent_st.st_mode &S_IFMT ) ==S_IFDIR ) + act =DA_VISIT ; +else if ((dent_st.st_mode &S_IFMT ) ==S_IFLNK ) + act =DA_IGNORE ; +else + act =DA_READ ; + } + +if (act ==DA_VISIT ) +visit_dir (dent_path); +else if (act ==DA_READ ) +rep_sched_work (dent_path, repeat); + } + +if (closedir (dir)) +tst_res (TINFO |TERRNO ,"closedir(%s)" , path); +} + +static void run (void ) +{ +spawn_workers (); +visit_dir (root_dir); +stop_workers (); + +tst_reap_children ();testcases/kernel/fs/read_all/read_all.c line 394
Quoted 10 lines
testcases/kernel/fs/read_all/read_all.c new file +10
-0+tst_res (TPASS ,"Finished reading files" ); +} + +static struct tst_test test = { + .options = options, + .setup = setup, + .cleanup = cleanup, + .test_all = run, + .forks_child =1 , +};The rest looks good to me.
Signature · 2 lines
Thank you, Richard.
PATCH v2 1/1 Add read_all file systems test
Hi!
Quoted 9 lines
Quoted 5 lines
testcases/kernel/fs/read_all/read_all.c line 232
Quoted 2 lines
testcases/kernel/fs/read_all/read_all.c new file +2
-0+queue_destroy (q,1 ); +fflush (stdout);What are trying to flush here? The
tst_res()messages are printed into the stderr btw.I found that the pass message was being written after some of the childrens' information messages. Calling fflush on stdout prevents that from happening even though it is the wrong fd...
You mean the tst_res(TPASS, ...) called after the tst_reap_children() ?
That is really strange since at that time the processes had called exit() already and the buffers should have been flushed at that point. I.e. if we haven't flushed it in the worker_run() the exit() should take care of that, which is guaranteed to run before we return from the tst_reap_children().
Signature · 2 lines
Cyril Hrubis [email protected]
PATCH v2 1/1 Add read_all file systems test
Hello,
Quoted 11 lines · Cyril Hrubis
Hi!
Quoted 9 lines
Quoted 5 lines
testcases/kernel/fs/read_all/read_all.c line 232
Quoted 2 lines
testcases/kernel/fs/read_all/read_all.c new file +2
-0+queue_destroy (q,1 ); +fflush (stdout);What are trying to flush here? The
tst_res()messages are printed into the stderr btw.I found that the pass message was being written after some of the childrens' information messages. Calling fflush on stdout prevents that from happening even though it is the wrong fd...
Quoted 7 lines
You mean the
tst_res(TPASS, ...)called after thetst_reap_children()?That is really strange since at that time the processes had called
exit()already and the buffers should have been flushed at that point. I.e. if we haven't flushed it in theworker_run()theexit()should take care of that, which is guaranteed to run before we return from thetst_reap_children().
It only happens on older products, so it could be removed if you are not concerned about that? I'm not sure if investigating it further would be useful.
Also I found a bug when pushing to a full queue, so I need to do another patch revision anyway.
--
Thank you,
Richard.