| Message ID | 20240530044029.15474-1-wegao@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v1] msync04.c: Use direct IO to verify the data is stored on disk | expand |
On Thu 30-05-24 00:40:29, Wei Gao wrote: > Signed-off-by: Wei Gao <wegao@suse.com> > Suggested-by: Jan Kara <jack@suse.cz> Looks good to me! Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > 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"); > + test_fd = SAFE_OPEN("msync04/testfile", O_RDONLY | O_DIRECT); > + SAFE_READ(0, test_fd, buffer, getpagesize()); > + > + char *char_buffer = (char *)buffer; > + > + 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"); > + else > + tst_res(TFAIL, "write file failed"); > +} > + > +static void verify_dirty(void) > { > - char buffer[20]; > + TST_EXP_PASS_SILENT(msync(mmaped_area, pagesize, MS_SYNC)); > + > + if (TST_RET == 0 && !get_dirty_bit(mmaped_area)) > + tst_res(TPASS, "msync() verify dirty page ok"); > + else > + tst_res(TFAIL, "msync() verify dirty page failed"); > +} > > +static void test_msync(void) > +{ > test_fd = SAFE_OPEN("msync04/testfile", O_CREAT | O_TRUNC | O_RDWR, > 0644); > SAFE_WRITE(SAFE_WRITE_ANY, test_fd, STRING_TO_WRITE, sizeof(STRING_TO_WRITE) - 1); > @@ -55,27 +81,11 @@ static void test_msync(void) > SAFE_CLOSE(test_fd); > mmaped_area[8] = 'B'; > > - if (!get_dirty_bit(mmaped_area)) { > - 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; > - } > - if (get_dirty_bit(mmaped_area)) > - tst_res(TFAIL, "msync() failed to write dirty page despite succeeding"); > - else > - tst_res(TPASS, "msync() working correctly"); > - } > - > -clean: > + if (!get_dirty_bit(mmaped_area)) > + verify_mmaped(); > + else > + verify_dirty(); > + > SAFE_MUNMAP(mmaped_area, pagesize); > mmaped_area = NULL; > } > -- > 2.35.3 >
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.
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"); + test_fd = SAFE_OPEN("msync04/testfile", O_RDONLY | O_DIRECT); + SAFE_READ(0, test_fd, buffer, getpagesize()); + + char *char_buffer = (char *)buffer; + + 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"); + else + tst_res(TFAIL, "write file failed"); +} + +static void verify_dirty(void) { - char buffer[20]; + TST_EXP_PASS_SILENT(msync(mmaped_area, pagesize, MS_SYNC)); + + if (TST_RET == 0 && !get_dirty_bit(mmaped_area)) + tst_res(TPASS, "msync() verify dirty page ok"); + else + tst_res(TFAIL, "msync() verify dirty page failed"); +} +static void test_msync(void) +{ test_fd = SAFE_OPEN("msync04/testfile", O_CREAT | O_TRUNC | O_RDWR, 0644); SAFE_WRITE(SAFE_WRITE_ANY, test_fd, STRING_TO_WRITE, sizeof(STRING_TO_WRITE) - 1); @@ -55,27 +81,11 @@ static void test_msync(void) SAFE_CLOSE(test_fd); mmaped_area[8] = 'B'; - if (!get_dirty_bit(mmaped_area)) { - 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; - } - if (get_dirty_bit(mmaped_area)) - tst_res(TFAIL, "msync() failed to write dirty page despite succeeding"); - else - tst_res(TPASS, "msync() working correctly"); - } - -clean: + if (!get_dirty_bit(mmaped_area)) + verify_mmaped(); + else + verify_dirty(); + SAFE_MUNMAP(mmaped_area, pagesize); mmaped_area = NULL; }
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(-)