On Tue, Oct 25, 2022 at 10:49 AM Shinichiro Kawasaki <shinichiro.kawasaki@xxxxxxx> wrote: > > On Oct 24, 2022 / 14:13, Yi Zhang wrote: > > This commit alo updated nvme/012 nvme/013 nvme/035 to pass the size > > Let's note why we do this. How about this descprtion? > > Change fio I/O size of nvme/012,013,035 from 950m to 900m, since recent > change increased the xfs log size and it caused fio failure with I/O > size 950m. > > Also add size parameter to _run_fio_verify_io. This allows to move the > fio I/O size definition from common/xfs to the test case, so that device > size and fio I/O size are both defined at single place. > > I think the commit title needs to reflect the motivations above, like: Agree, updated the title and description in V2. > > nvme/012,013,035: change fio I/O size and move size definition place > > > parameter to _xfs_run_fio_verify_io > > > > Signed-off-by: Yi Zhang <yi.zhang@xxxxxxxxxx> > > How about to add "Link:" tag to refer our discussion? > > Link: https://lore.kernel.org/linux-block/20221019051244.810755-1-yi.zhang@xxxxxxxxxx/ Added in V2. Again, thanks Shinichiro for the review. > > > > --- > > common/xfs | 3 ++- > > tests/nvme/012 | 2 +- > > tests/nvme/013 | 2 +- > > tests/nvme/035 | 9 ++++++++- > > 4 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/common/xfs b/common/xfs > > index 846a5ef..2c5d961 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -23,10 +23,11 @@ _xfs_mkfs_and_mount() { > > _xfs_run_fio_verify_io() { > > local mount_dir="/mnt/blktests" > > local bdev=$1 > > + local sz=$2 > > > > _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 > > > > - _run_fio_verify_io --size=950m --directory="${mount_dir}/" > > + _run_fio_verify_io --size="$sz" --directory="${mount_dir}/" > > > > umount "${mount_dir}" >> "${FULL}" 2>&1 > > rm -fr "${mount_dir}" > > diff --git a/tests/nvme/012 b/tests/nvme/012 > > index c9d2438..e60082c 100755 > > --- a/tests/nvme/012 > > +++ b/tests/nvme/012 > > @@ -44,7 +44,7 @@ test() { > > cat "/sys/block/${nvmedev}n1/uuid" > > cat "/sys/block/${nvmedev}n1/wwid" > > > > - _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" "900m" > > > > _nvme_disconnect_subsys "${subsys_name}" > > > > diff --git a/tests/nvme/013 b/tests/nvme/013 > > index 265b696..9d60a7d 100755 > > --- a/tests/nvme/013 > > +++ b/tests/nvme/013 > > @@ -41,7 +41,7 @@ test() { > > cat "/sys/block/${nvmedev}n1/uuid" > > cat "/sys/block/${nvmedev}n1/wwid" > > > > - _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" "900m" > > > > _nvme_disconnect_subsys "${subsys_name}" > > > > diff --git a/tests/nvme/035 b/tests/nvme/035 > > index ee78a75..31de0d1 100755 > > --- a/tests/nvme/035 > > +++ b/tests/nvme/035 > > @@ -21,14 +21,21 @@ test_device() { > > local ctrldev > > local nsdev > > local port > > + local test_dev_sz > > > > echo "Running ${TEST_NAME}" > > > > _setup_nvmet > > port=$(_nvmet_passthru_target_setup "${subsys}") > > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > + test_dev_sz=$(_get_test_dev_size_mb) > > > > - _xfs_run_fio_verify_io "${nsdev}" > > + if (( "$test_dev_sz" < 1024 )); then > > + echo "Test dev: $TEST_DEV should at leat 1024m" > > + return 1 > > + > > + fi > > As I commented on the second patch, I suggest to move this device size check > part to the second patch. Done in V2. > > Other changes looks good tome. > > > + _xfs_run_fio_verify_io "${nsdev}" "900m" > > > > _nvme_disconnect_subsys "${subsys}" > > _nvmet_passthru_target_cleanup "${port}" "${subsys}" > > -- > > 2.34.1 > > > > -- > Shin'ichiro Kawasaki > -- Best Regards, Yi Zhang
