Re: [LTP] [PATCH v1] msync04: Check disk content if dirty bit check failed

[Petr Vorel <pvorel@suse.cz>]

Hi Jan, Wei,

> msync04 test is inherently racy and nothing guarantees that the page
> is not written out before get_dirty_page() manages to read the page state.
> Hence the test should be reworked to verify the page contents is on disk
> when it finds dirty bit isn't set anymore...

@Jan some time ago [1] you asked to:

	mmap file, write to mmap, msync, abort fs, mount fs again, check the data is
	there.

Could you please have a look if this is enough? Or is it aborting fs and mounting
again necessary?

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20220125121746.wrs4254pfs2mwexb@quack3.lan/

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/msync/msync04.c | 34 ++++++++++++++---------
>  1 file changed, 21 insertions(+), 13 deletions(-)

> diff --git a/testcases/kernel/syscalls/msync/msync04.c b/testcases/kernel/syscalls/msync/msync04.c
> index 72ddc27a4..c296c8b20 100644
> --- a/testcases/kernel/syscalls/msync/msync04.c
> +++ b/testcases/kernel/syscalls/msync/msync04.c
> @@ -46,6 +46,7 @@ uint64_t get_dirty_bit(void *data)
>  static void test_msync(void)
>  {
>  	uint64_t dirty;
> +	char buffer[20];

>  	test_fd = SAFE_OPEN("msync04/testfile", O_CREAT | O_TRUNC | O_RDWR,
>  		0644);
> @@ -56,20 +57,27 @@ static void test_msync(void)
>  	mmaped_area[8] = 'B';
>  	dirty = get_dirty_bit(mmaped_area);
>  	if (!dirty) {
> -		tst_res(TFAIL, "Expected dirty bit to be set after writing to"
> -				" mmap()-ed area");
> -		goto clean;
> -	}
> -	if (msync(mmaped_area, pagesize, MS_SYNC) < 0) {
> -		tst_res(TFAIL | TERRNO, "msync() failed");
> -		goto clean;
> +		tst_res(TINFO, "Not see dirty bit so we check content of file instead");
> +		test_fd = SAFE_OPEN("msync04/testfile", O_RDONLY);
> +		SAFE_READ(0, test_fd, buffer, 9);
> +		if (buffer[8] == 'B')
> +			tst_res(TCONF, "write file ok but msync couldn't be tested"
> +				" because the storage was written to too quickly");
> +		else
> +			tst_res(TFAIL, "write file failed");
> +	} else {
> +		if (msync(mmaped_area, pagesize, MS_SYNC) < 0) {
> +			tst_res(TFAIL | TERRNO, "msync() failed");
> +			goto clean;
> +		}
> +		dirty = get_dirty_bit(mmaped_area);
> +		if (dirty)
> +			tst_res(TFAIL, "msync() failed to write dirty page despite"
> +					" succeeding");
> +		else
> +			tst_res(TPASS, "msync() working correctly");
> +
>  	}
> -	dirty = get_dirty_bit(mmaped_area);
> -	if (dirty)
> -		tst_res(TFAIL, "msync() failed to write dirty page despite"
> -				" succeeding");
> -	else
> -		tst_res(TPASS, "msync() working correctly");

>  clean:
>  	SAFE_MUNMAP(mmaped_area, pagesize);

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