Re: [LTP] [PATCH v1] msync04.c: Use direct IO to verify the data is stored on disk

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: Wei Gao <wegao@suse.com>
> Suggested-by: Jan Kara <jack@suse.cz>
> ---
>  testcases/kernel/syscalls/msync/msync04.c | 56 +++++++++++++----------
>  1 file changed, 33 insertions(+), 23 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/msync/msync04.c b/testcases/kernel/syscalls/msync/msync04.c
> index 1718bd7dc..c0580d1b0 100644
> --- a/testcases/kernel/syscalls/msync/msync04.c
> +++ b/testcases/kernel/syscalls/msync/msync04.c
> @@ -11,6 +11,7 @@
>   * is no longer dirty after msync() call.
>   */
>  
> +#define _GNU_SOURCE
>  #include <errno.h>
>  #include "tst_test.h"
>  
> @@ -43,10 +44,35 @@ static uint64_t get_dirty_bit(void *data)
>  	return pageflag_entry & (1ULL << 4);
>  }
>  
> -static void test_msync(void)
> +static void verify_mmaped(void)
> +{
> +	void *buffer = SAFE_MEMALIGN(getpagesize(), getpagesize());
> +
> +	tst_res(TINFO, "Not see dirty bit so we check content of file instead");
                         ^
			Haven't seen dirty...

> +	test_fd = SAFE_OPEN("msync04/testfile", O_RDONLY | O_DIRECT);
> +	SAFE_READ(0, test_fd, buffer, getpagesize());
> +
> +	char *char_buffer = (char *)buffer;

You can declare the buffer directly as char * instead, there is no need
for this indirection.

> +	if (char_buffer[8] == 'B')
> +		tst_res(TCONF, "write file ok but msync couldn't be tested"
> +				" because the storage was written to too quickly");

This could be shorter and stil to the point:

"Write was too fast, couldn't test msync()"

> +	else
> +		tst_res(TFAIL, "write file failed");

We should free() allocated the memory here.


Other than these minor issues the rest looks good to me.

You can add my Reviewed-by: once these issues are fixed.

-- 
Cyril Hrubis
chrubis@suse.cz

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