| Message ID | 20230421060505.10132-4-dwagner@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nvme testsuite runtime optimization | expand |
On 4/21/23 08:04, Daniel Wagner wrote: > Make the size argument optional by reading the filesystem info. The > caller doesn't have to guess (or calculate) how big the max IO size. > The log data structure of XFS is reducing the capacity. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > common/xfs | 6 ++++++ > tests/nvme/012 | 2 +- > tests/nvme/013 | 2 +- > tests/nvme/035 | 2 +- > 4 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/common/xfs b/common/xfs > index 2c5d96164ac1..ec35599e017b 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -27,6 +27,12 @@ _xfs_run_fio_verify_io() { > > _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 > > + if [[ -z "${sz}" ]]; then > + local avail > + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" df --output=avail "${mount_dir}" | tail -1 > + sz="$(printf "%d" $((avail / 1024 - 1 )))m" sz=$((avail / 1024 - 1)) > + fi > + > _run_fio_verify_io --size="$sz" --directory="${mount_dir}/" _run_fio_verify_io --size="${sz}m" --directory="${mount_dir}/" > > umount "${mount_dir}" >> "${FULL}" 2>&1 > diff --git a/tests/nvme/012 b/tests/nvme/012 > index e60082c2e751..c9d24388306d 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" "900m" > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > _nvme_disconnect_subsys "${subsys_name}" > > diff --git a/tests/nvme/013 b/tests/nvme/013 > index 9d60a7df4577..265b6968fd34 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" "900m" > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > _nvme_disconnect_subsys "${subsys_name}" > > diff --git a/tests/nvme/035 b/tests/nvme/035 > index eb1024edddbf..8b485bc8e682 100755 > --- a/tests/nvme/035 > +++ b/tests/nvme/035 > @@ -32,7 +32,7 @@ test_device() { > port=$(_nvmet_passthru_target_setup "${subsys}") > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > - _xfs_run_fio_verify_io "${nsdev}" "900m" > + _xfs_run_fio_verify_io "${nsdev}" > > _nvme_disconnect_subsys "${subsys}" > _nvmet_passthru_target_cleanup "${port}" "${subsys}" Otherwise looks good. Cheers, Hannes
On Fri, Apr 21, 2023 at 08:27:35AM +0200, Hannes Reinecke wrote: > On 4/21/23 08:04, Daniel Wagner wrote: > > Make the size argument optional by reading the filesystem info. The > > caller doesn't have to guess (or calculate) how big the max IO size. > > The log data structure of XFS is reducing the capacity. > > > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > --- > > common/xfs | 6 ++++++ > > tests/nvme/012 | 2 +- > > tests/nvme/013 | 2 +- > > tests/nvme/035 | 2 +- > > 4 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/common/xfs b/common/xfs > > index 2c5d96164ac1..ec35599e017b 100644 > > --- a/common/xfs > > +++ b/common/xfs > > @@ -27,6 +27,12 @@ _xfs_run_fio_verify_io() { > > _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 > > + if [[ -z "${sz}" ]]; then > > + local avail > > + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" > > df --output=avail "${mount_dir}" | tail -1 Sure, don't think it matters. > > + sz="$(printf "%d" $((avail / 1024 - 1 )))m" > > sz=$((avail / 1024 - 1)) I tried this but the devision is likely to return a floating point which fio doesn't like. Is there a way to tell bash to do a pure integer devision?
On Apr 21, 2023 / 08:54, Daniel Wagner wrote: > On Fri, Apr 21, 2023 at 08:27:35AM +0200, Hannes Reinecke wrote: > > On 4/21/23 08:04, Daniel Wagner wrote: > > > Make the size argument optional by reading the filesystem info. The > > > caller doesn't have to guess (or calculate) how big the max IO size. > > > The log data structure of XFS is reducing the capacity. > > > > > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > > > --- > > > common/xfs | 6 ++++++ > > > tests/nvme/012 | 2 +- > > > tests/nvme/013 | 2 +- > > > tests/nvme/035 | 2 +- > > > 4 files changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/common/xfs b/common/xfs > > > index 2c5d96164ac1..ec35599e017b 100644 > > > --- a/common/xfs > > > +++ b/common/xfs > > > @@ -27,6 +27,12 @@ _xfs_run_fio_verify_io() { > > > _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 > > > + if [[ -z "${sz}" ]]; then > > > + local avail > > > + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" > > > > df --output=avail "${mount_dir}" | tail -1 > > Sure, don't think it matters. > > > > + sz="$(printf "%d" $((avail / 1024 - 1 )))m" > > > > sz=$((avail / 1024 - 1)) > > I tried this but the devision is likely to return a floating point which fio > doesn't like. Is there a way to tell bash to do a pure integer devision? Hmm, AFAIK, bash arithmetic supports integer only. I tried below, and bash did not return floating value... $ avail=90000; echo $((avail/1024)) 87 Assuming bash arithmetic supports integer only, -1 will not be required in the calculation.
On Apr 21, 2023 / 08:04, Daniel Wagner wrote: > Make the size argument optional by reading the filesystem info. The > caller doesn't have to guess (or calculate) how big the max IO size. > The log data structure of XFS is reducing the capacity. > > Signed-off-by: Daniel Wagner <dwagner@suse.de> > --- > common/xfs | 6 ++++++ > tests/nvme/012 | 2 +- > tests/nvme/013 | 2 +- > tests/nvme/035 | 2 +- > 4 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/common/xfs b/common/xfs > index 2c5d96164ac1..ec35599e017b 100644 > --- a/common/xfs > +++ b/common/xfs > @@ -27,6 +27,12 @@ _xfs_run_fio_verify_io() { > > _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 > > + if [[ -z "${sz}" ]]; then > + local avail > + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" > + sz="$(printf "%d" $((avail / 1024 - 1 )))m" > + fi > + > _run_fio_verify_io --size="$sz" --directory="${mount_dir}/" > > umount "${mount_dir}" >> "${FULL}" 2>&1 > diff --git a/tests/nvme/012 b/tests/nvme/012 > index e60082c2e751..c9d24388306d 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" "900m" > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > _nvme_disconnect_subsys "${subsys_name}" > > diff --git a/tests/nvme/013 b/tests/nvme/013 > index 9d60a7df4577..265b6968fd34 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" "900m" > + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" > > _nvme_disconnect_subsys "${subsys_name}" > As for nvme/012 and nvme/013, I observed the I/O size changes from 900m to 920m with this patch. This condition looks better for testing point of view. Good. > diff --git a/tests/nvme/035 b/tests/nvme/035 > index eb1024edddbf..8b485bc8e682 100755 > --- a/tests/nvme/035 > +++ b/tests/nvme/035 > @@ -32,7 +32,7 @@ test_device() { > port=$(_nvmet_passthru_target_setup "${subsys}") > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > - _xfs_run_fio_verify_io "${nsdev}" "900m" > + _xfs_run_fio_verify_io "${nsdev}" On the other hand, this change for nvme/035 does not look good. It runs the test on TEST_DEV, which may take very long time without TIMEOUT config. > > _nvme_disconnect_subsys "${subsys}" > _nvmet_passthru_target_cleanup "${port}" "${subsys}" > -- > 2.40.0 >
On Fri, Apr 21, 2023 at 08:27:35AM +0200, Hannes Reinecke wrote: > > + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" > > df --output=avail "${mount_dir}" | tail -1 ok. > > + sz="$(printf "%d" $((avail / 1024 - 1 )))m" > > sz=$((avail / 1024 - 1)) > > > + fi > > + > > _run_fio_verify_io --size="$sz" --directory="${mount_dir}/" > > _run_fio_verify_io --size="${sz}m" --directory="${mount_dir}/" $sz might already contain the 'm', so can't do this here.
> Hmm, AFAIK, bash arithmetic supports integer only. I tried below, and bash did > not return floating value... > > $ avail=90000; echo $((avail/1024)) > 87 > > Assuming bash arithmetic supports integer only, -1 will not be required in the > calculation. Can't remember how I ended up with the above. Anyway, works just fine without the printf.
> --- a/tests/nvme/035 > > +++ b/tests/nvme/035 > > @@ -32,7 +32,7 @@ test_device() { > > port=$(_nvmet_passthru_target_setup "${subsys}") > > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > > > - _xfs_run_fio_verify_io "${nsdev}" "900m" > > + _xfs_run_fio_verify_io "${nsdev}" > > On the other hand, this change for nvme/035 does not look good. It runs the > test on TEST_DEV, which may take very long time without TIMEOUT config. I'll add the nvme_img_size argument here instead (nvme: Make test image size configurable)
On May 02, 2023 / 15:23, Daniel Wagner wrote: > > --- a/tests/nvme/035 > > > +++ b/tests/nvme/035 > > > @@ -32,7 +32,7 @@ test_device() { > > > port=$(_nvmet_passthru_target_setup "${subsys}") > > > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > > > > > - _xfs_run_fio_verify_io "${nsdev}" "900m" > > > + _xfs_run_fio_verify_io "${nsdev}" > > > > On the other hand, this change for nvme/035 does not look good. It runs the > > test on TEST_DEV, which may take very long time without TIMEOUT config. > > I'll add the nvme_img_size argument here instead (nvme: Make test image size > configurable) If TEST_DEV has the size same as nvme_img_size, xfs log data will consume some part of the TEST_DEV, then _xfs_run_fio_verify_io with nvme_img_size will fail. I think the size argument of _xfs_run_fio_verify_io should be, min(size of TEST_DEV, nvm_img_size) - log data size of xfs But I'm not sure if we can do this calculation correctly. If the calculation is not possible, it would be the better to leave the hard coded constants (1GB for TEST_DEV size and 900mb as fio I/O size) in this test case, because nvme/035 is rather unique in the nvme group, which uses TEST_DEV.
On Wed, May 03, 2023 at 04:04:50AM +0000, Shinichiro Kawasaki wrote: > On May 02, 2023 / 15:23, Daniel Wagner wrote: > > > --- a/tests/nvme/035 > > > > +++ b/tests/nvme/035 > > > > @@ -32,7 +32,7 @@ test_device() { > > > > port=$(_nvmet_passthru_target_setup "${subsys}") > > > > nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") > > > > > > > > - _xfs_run_fio_verify_io "${nsdev}" "900m" > > > > + _xfs_run_fio_verify_io "${nsdev}" > > > > > > On the other hand, this change for nvme/035 does not look good. It runs the > > > test on TEST_DEV, which may take very long time without TIMEOUT config. > > > > I'll add the nvme_img_size argument here instead (nvme: Make test image size > > configurable) > > If TEST_DEV has the size same as nvme_img_size, xfs log data will consume some > part of the TEST_DEV, then _xfs_run_fio_verify_io with nvme_img_size will fail. > > I think the size argument of _xfs_run_fio_verify_io should be, > > min(size of TEST_DEV, nvm_img_size) - log data size of xfs > > But I'm not sure if we can do this calculation correctly. > > If the calculation is not possible, it would be the better to leave the hard > coded constants (1GB for TEST_DEV size and 900mb as fio I/O size) in this test > case, because nvme/035 is rather unique in the nvme group, which uses TEST_DEV. I've solved this by extending _xfs_run_fio_verify_io() to limit the max size of the io job: _xfs_run_fio_verify_io() { local mount_dir="/mnt/blktests" local bdev=$1 local sz=$2 local sz_mb local avail local avail_mb _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 avail="$(df --output=avail "${mount_dir}" | tail -1)" avail_mb="$((avail / 1024))" if [[ -z "${sz}" ]]; then sz_mb="${avail_mb}" else sz_mb="$(convert_to_mb "${sz}")" if [[ "${sz_mb}" -gt "${avail_mb}" ]]; then sz_mb="${avail_mb}" fi fi _run_fio_verify_io --size="${sz_mb}m" --directory="${mount_dir}/" umount "${mount_dir}" >> "${FULL}" 2>&1 rm -fr "${mount_dir}" } Anyway, I'll send out the updated series shortly
diff --git a/common/xfs b/common/xfs index 2c5d96164ac1..ec35599e017b 100644 --- a/common/xfs +++ b/common/xfs @@ -27,6 +27,12 @@ _xfs_run_fio_verify_io() { _xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1 + if [[ -z "${sz}" ]]; then + local avail + avail="$(df --output=avail "${mount_dir}" | awk 'NR==2 {print $1}')" + sz="$(printf "%d" $((avail / 1024 - 1 )))m" + fi + _run_fio_verify_io --size="$sz" --directory="${mount_dir}/" umount "${mount_dir}" >> "${FULL}" 2>&1 diff --git a/tests/nvme/012 b/tests/nvme/012 index e60082c2e751..c9d24388306d 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" "900m" + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" _nvme_disconnect_subsys "${subsys_name}" diff --git a/tests/nvme/013 b/tests/nvme/013 index 9d60a7df4577..265b6968fd34 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" "900m" + _xfs_run_fio_verify_io "/dev/${nvmedev}n1" _nvme_disconnect_subsys "${subsys_name}" diff --git a/tests/nvme/035 b/tests/nvme/035 index eb1024edddbf..8b485bc8e682 100755 --- a/tests/nvme/035 +++ b/tests/nvme/035 @@ -32,7 +32,7 @@ test_device() { port=$(_nvmet_passthru_target_setup "${subsys}") nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}") - _xfs_run_fio_verify_io "${nsdev}" "900m" + _xfs_run_fio_verify_io "${nsdev}" _nvme_disconnect_subsys "${subsys}" _nvmet_passthru_target_cleanup "${port}" "${subsys}"
Make the size argument optional by reading the filesystem info. The caller doesn't have to guess (or calculate) how big the max IO size. The log data structure of XFS is reducing the capacity. Signed-off-by: Daniel Wagner <dwagner@suse.de> --- common/xfs | 6 ++++++ tests/nvme/012 | 2 +- tests/nvme/013 | 2 +- tests/nvme/035 | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-)