diff mbox series

[5/7] fsstress: allow afsync on directories too

Message ID 20190328185458.29033-1-fdmanana@kernel.org (mailing list archive)
State New, archived
Headers show
Series [1/7] fsstress: rename setxattr operation to chproj | expand

Commit Message

Filipe Manana March 28, 2019, 6:54 p.m. UTC 
From: Filipe Manana <fdmanana@suse.com>

Currently the afsync function can only be performed against regular files.
Allow it to operate on directories too, to increase test coverage and allow
for chances of finding bugs in the filesystem implementation of fsync
against directories.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 ltp/fsstress.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Nikolay Borisov March 28, 2019, 9:36 p.m. UTC | #1 
On 28.03.19 г. 20:54 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Currently the afsync function can only be performed against regular files.
> Allow it to operate on directories too, to increase test coverage and allow
> for chances of finding bugs in the filesystem implementation of fsync
> against directories.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  ltp/fsstress.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index ffe78ef7..0fb9e399 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -1767,15 +1767,21 @@ afsync_f(int opno, long r)
>  	struct iocb	iocb;
>  	struct iocb	*iocbs[] = { &iocb };
>  	struct io_event	event;
> +	DIR             *dir = NULL;
>  
>  	init_pathname(&f);
> -	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> +	if (!get_fname(FT_REGFILE | FT_DIRm, r, &f, NULL, NULL, &v)) {

s/FT_DIRm/FT_DIR/

>  		if (v)
>  			printf("%d/%d: afsync - no filename\n", procid, opno);
>  		free_pathname(&f);
>  		return;
>  	}
>  	fd = open_path(&f, O_WRONLY | O_DIRECT);
> +	if (fd < 0 && errno == EISDIR) {
> +		dir = opendir_path(&f);
> +		if (dir)
> +			fd = dirfd(dir);
> +	}
>  	e = fd < 0 ? errno : 0;
>  	check_cwd();
>  	if (fd < 0) {
> @@ -1783,6 +1789,8 @@ afsync_f(int opno, long r)
>  			printf("%d/%d: afsync - open %s failed %d\n",
>  			       procid, opno, f.path, e);
>  		free_pathname(&f);
> +		if (dir)
> +			closedir(dir);
>  		return;
>  	}
>  
> @@ -1791,24 +1799,24 @@ afsync_f(int opno, long r)
>  		if (v)
>  			printf("%d/%d: afsync - io_submit %s %d\n",
>  			       procid, opno, f.path, e);
> -		free_pathname(&f);
> -		close(fd);
> -		return;
> +		goto out;
>  	}
>  	if ((e = io_getevents(io_ctx, 1, 1, &event, NULL)) != 1) {
>  		if (v)
>  			printf("%d/%d: afsync - io_getevents failed %d\n",
>  			       procid, opno, e);
> -		free_pathname(&f);
> -		close(fd);
> -		return;
> +		goto out;
>  	}
>  
>  	e = event.res2;
>  	if (v)
>  		printf("%d/%d: afsync %s %d\n", procid, opno, f.path, e);
> +out:
>  	free_pathname(&f);
> -	close(fd);
> +	if (dir)
> +		closedir(dir);
> +	else
> +		close(fd);
>  #endif
>  }
>  
>
Nikolay Borisov March 28, 2019, 9:37 p.m. UTC | #2 
On 28.03.19 г. 23:36 ч., Nikolay Borisov wrote:
> 
> 
> On 28.03.19 г. 20:54 ч., fdmanana@kernel.org wrote:
>> From: Filipe Manana <fdmanana@suse.com>
>>
>> Currently the afsync function can only be performed against regular files.
>> Allow it to operate on directories too, to increase test coverage and allow
>> for chances of finding bugs in the filesystem implementation of fsync
>> against directories.
>>
>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>> ---
>>  ltp/fsstress.c | 24 ++++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
>> index ffe78ef7..0fb9e399 100644
>> --- a/ltp/fsstress.c
>> +++ b/ltp/fsstress.c
>> @@ -1767,15 +1767,21 @@ afsync_f(int opno, long r)
>>  	struct iocb	iocb;
>>  	struct iocb	*iocbs[] = { &iocb };
>>  	struct io_event	event;
>> +	DIR             *dir = NULL;
>>  
>>  	init_pathname(&f);
>> -	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
>> +	if (!get_fname(FT_REGFILE | FT_DIRm, r, &f, NULL, NULL, &v)) {
> 
> s/FT_DIRm/FT_DIR/

Bummer, FT_DIRm is defined to FT_DIR.... disregard this...

> 
>>  		if (v)
>>  			printf("%d/%d: afsync - no filename\n", procid, opno);
>>  		free_pathname(&f);
>>  		return;
>>  	}
>>  	fd = open_path(&f, O_WRONLY | O_DIRECT);
>> +	if (fd < 0 && errno == EISDIR) {
>> +		dir = opendir_path(&f);
>> +		if (dir)
>> +			fd = dirfd(dir);
>> +	}
>>  	e = fd < 0 ? errno : 0;
>>  	check_cwd();
>>  	if (fd < 0) {
>> @@ -1783,6 +1789,8 @@ afsync_f(int opno, long r)
>>  			printf("%d/%d: afsync - open %s failed %d\n",
>>  			       procid, opno, f.path, e);
>>  		free_pathname(&f);
>> +		if (dir)
>> +			closedir(dir);
>>  		return;
>>  	}
>>  
>> @@ -1791,24 +1799,24 @@ afsync_f(int opno, long r)
>>  		if (v)
>>  			printf("%d/%d: afsync - io_submit %s %d\n",
>>  			       procid, opno, f.path, e);
>> -		free_pathname(&f);
>> -		close(fd);
>> -		return;
>> +		goto out;
>>  	}
>>  	if ((e = io_getevents(io_ctx, 1, 1, &event, NULL)) != 1) {
>>  		if (v)
>>  			printf("%d/%d: afsync - io_getevents failed %d\n",
>>  			       procid, opno, e);
>> -		free_pathname(&f);
>> -		close(fd);
>> -		return;
>> +		goto out;
>>  	}
>>  
>>  	e = event.res2;
>>  	if (v)
>>  		printf("%d/%d: afsync %s %d\n", procid, opno, f.path, e);
>> +out:
>>  	free_pathname(&f);
>> -	close(fd);
>> +	if (dir)
>> +		closedir(dir);
>> +	else
>> +		close(fd);
>>  #endif
>>  }
>>  
>>
>
Dave Chinner March 28, 2019, 9:55 p.m. UTC | #3 
On Thu, Mar 28, 2019 at 06:54:58PM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Currently the afsync function can only be performed against regular files.
> Allow it to operate on directories too, to increase test coverage and allow
> for chances of finding bugs in the filesystem implementation of fsync
> against directories.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>  ltp/fsstress.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index ffe78ef7..0fb9e399 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -1767,15 +1767,21 @@ afsync_f(int opno, long r)
>  	struct iocb	iocb;
>  	struct iocb	*iocbs[] = { &iocb };
>  	struct io_event	event;
> +	DIR             *dir = NULL;
>  
>  	init_pathname(&f);
> -	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
> +	if (!get_fname(FT_REGFILE | FT_DIRm, r, &f, NULL, NULL, &v)) {
>  		if (v)
>  			printf("%d/%d: afsync - no filename\n", procid, opno);
>  		free_pathname(&f);
>  		return;
>  	}
>  	fd = open_path(&f, O_WRONLY | O_DIRECT);
> +	if (fd < 0 && errno == EISDIR) {
> +		dir = opendir_path(&f);
> +		if (dir)
> +			fd = dirfd(dir);
> +	}

You've added this pattern to about 5 functions now. Please factor
the whole getname/open thing into a helper that can be used
everywhere....

>  	e = fd < 0 ? errno : 0;
>  	check_cwd();
>  	if (fd < 0) {
> @@ -1783,6 +1789,8 @@ afsync_f(int opno, long r)
>  			printf("%d/%d: afsync - open %s failed %d\n",
>  			       procid, opno, f.path, e);
>  		free_pathname(&f);
> +		if (dir)
> +			closedir(dir);
>  		return;
>  	}
>  
> @@ -1791,24 +1799,24 @@ afsync_f(int opno, long r)
>  		if (v)
>  			printf("%d/%d: afsync - io_submit %s %d\n",
>  			       procid, opno, f.path, e);
> -		free_pathname(&f);
> -		close(fd);
> -		return;
> +		goto out;
>  	}
>  	if ((e = io_getevents(io_ctx, 1, 1, &event, NULL)) != 1) {
>  		if (v)
>  			printf("%d/%d: afsync - io_getevents failed %d\n",
>  			       procid, opno, e);
> -		free_pathname(&f);
> -		close(fd);
> -		return;
> +		goto out;
>  	}
>  
>  	e = event.res2;
>  	if (v)
>  		printf("%d/%d: afsync %s %d\n", procid, opno, f.path, e);
> +out:
>  	free_pathname(&f);
> -	close(fd);
> +	if (dir)
> +		closedir(dir);
> +	else
> +		close(fd);

Same here for close.

Cheers,

Dave.
diff mbox series

Patch

diff --git a/ltp/fsstress.c b/ltp/fsstress.c
index ffe78ef7..0fb9e399 100644
--- a/ltp/fsstress.c
+++ b/ltp/fsstress.c
@@ -1767,15 +1767,21 @@  afsync_f(int opno, long r)
 	struct iocb	iocb;
 	struct iocb	*iocbs[] = { &iocb };
 	struct io_event	event;
+	DIR             *dir = NULL;
 
 	init_pathname(&f);
-	if (!get_fname(FT_REGFILE, r, &f, NULL, NULL, &v)) {
+	if (!get_fname(FT_REGFILE | FT_DIRm, r, &f, NULL, NULL, &v)) {
 		if (v)
 			printf("%d/%d: afsync - no filename\n", procid, opno);
 		free_pathname(&f);
 		return;
 	}
 	fd = open_path(&f, O_WRONLY | O_DIRECT);
+	if (fd < 0 && errno == EISDIR) {
+		dir = opendir_path(&f);
+		if (dir)
+			fd = dirfd(dir);
+	}
 	e = fd < 0 ? errno : 0;
 	check_cwd();
 	if (fd < 0) {
@@ -1783,6 +1789,8 @@  afsync_f(int opno, long r)
 			printf("%d/%d: afsync - open %s failed %d\n",
 			       procid, opno, f.path, e);
 		free_pathname(&f);
+		if (dir)
+			closedir(dir);
 		return;
 	}
 
@@ -1791,24 +1799,24 @@  afsync_f(int opno, long r)
 		if (v)
 			printf("%d/%d: afsync - io_submit %s %d\n",
 			       procid, opno, f.path, e);
-		free_pathname(&f);
-		close(fd);
-		return;
+		goto out;
 	}
 	if ((e = io_getevents(io_ctx, 1, 1, &event, NULL)) != 1) {
 		if (v)
 			printf("%d/%d: afsync - io_getevents failed %d\n",
 			       procid, opno, e);
-		free_pathname(&f);
-		close(fd);
-		return;
+		goto out;
 	}
 
 	e = event.res2;
 	if (v)
 		printf("%d/%d: afsync %s %d\n", procid, opno, f.path, e);
+out:
 	free_pathname(&f);
-	close(fd);
+	if (dir)
+		closedir(dir);
+	else
+		close(fd);
 #endif
 }