| Message ID | 20191021225719.211651-2-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a test that triggers blk_mq_update_nr_hw_queues() | expand |
Look good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> On 10/21/2019 03:57 PM, Bart Van Assche wrote: > Make it easy to use the uptime_s() function from block tests. > > Signed-off-by: Bart Van Assche<bvanassche@acm.org>
On Mon, Oct 21, 2019 at 03:57:18PM -0700, Bart Van Assche wrote: > Make it easy to use the uptime_s() function from block tests. > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > common/multipath-over-rdma | 9 +-------- > common/rc | 9 +++++++++ > tests/nvmeof-mp/rc | 2 +- > tests/srp/014 | 2 +- > tests/srp/rc | 2 +- > 5 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma > index 65ebb7b7f5f7..545a81e8c18e 100644 > --- a/common/multipath-over-rdma > +++ b/common/multipath-over-rdma > @@ -129,19 +129,12 @@ held_by() { > done > } > > -# System uptime in seconds. > -uptime_s() { > - local a b > - > - echo "$(</proc/uptime)" | { read -r a b && echo "${a%%.*}"; } > -} > - > # Sleep until either $1 seconds have elapsed or until the deadline $2 has been > # reached. Return 1 if and only if the deadline has been met. > sleep_until() { > local duration=$1 deadline=$2 u > > - u=$(uptime_s) > + u=$(_uptime_s) > if [ $((u + duration)) -le "$deadline" ]; then > sleep "$duration" > else > diff --git a/common/rc b/common/rc > index 41aee3aaa735..c00f2fe1f463 100644 > --- a/common/rc > +++ b/common/rc > @@ -246,3 +246,12 @@ _test_dev_is_partition() { > _filter_xfs_io_error() { > sed -e 's/^\(.*\)64\(: .*$\)/\1\2/' > } > + > +# System uptime in seconds. > +_uptime_s() { > + local a b > + > + echo "$(</proc/uptime)" | { What's wrong with cat /proc/uptime? Or even better, { read ... } < /proc/uptime > + read -r a b && echo "$b" >/dev/null && echo "${a%%.*}"; What's the point of the echo "$b" here? Seems like this could all be condensed to: { read -r s && echo "${s%%.*}" } < /proc/uptime But that's more cryptic than it needs to be. Can we just do: awk '{ print int($1) }' /proc/uptime
On 10/24/19 10:27 AM, Omar Sandoval wrote: > On Mon, Oct 21, 2019 at 03:57:18PM -0700, Bart Van Assche wrote: >> +# System uptime in seconds. >> +_uptime_s() { >> + local a b >> + >> + echo "$(</proc/uptime)" | { > > What's wrong with cat /proc/uptime? Or even better, > > { read ... } < /proc/uptime Hi Omar, As you probably know 'cat' triggers a fork() system call but echo $(<...) not. This is a performance optimization. Input redirection would also work. >> + read -r a b && echo "$b" >/dev/null && echo "${a%%.*}"; > > What's the point of the echo "$b" here? That echo "$b" statement suppresses a shellcheck warning about $b not being used. > Seems like this could all be condensed to: > > { read -r s && echo "${s%%.*}" } < /proc/uptime > > But that's more cryptic than it needs to be. Can we just do: > > awk '{ print int($1) }' /proc/uptime That's a valid alternative, but an alternative that triggers a fork() system call. I don't have a strong opinion about which alternative to choose. Do you perhaps have a preference? Thanks, Bart.
On Thu, Oct 24, 2019 at 10:41:49AM -0700, Bart Van Assche wrote: > On 10/24/19 10:27 AM, Omar Sandoval wrote: > > On Mon, Oct 21, 2019 at 03:57:18PM -0700, Bart Van Assche wrote: > > > +# System uptime in seconds. > > > +_uptime_s() { > > > + local a b > > > + > > > + echo "$(</proc/uptime)" | { > > > > What's wrong with cat /proc/uptime? Or even better, > > > > { read ... } < /proc/uptime > > Hi Omar, > > As you probably know 'cat' triggers a fork() system call but echo $(<...) > not. This is a performance optimization. Input redirection would also work. > > > > + read -r a b && echo "$b" >/dev/null && echo "${a%%.*}"; > > > > What's the point of the echo "$b" here? > > That echo "$b" statement suppresses a shellcheck warning about $b not being > used. > > > Seems like this could all be condensed to: > > > > { read -r s && echo "${s%%.*}" } < /proc/uptime > > > > But that's more cryptic than it needs to be. Can we just do: > > > > awk '{ print int($1) }' /proc/uptime > > That's a valid alternative, but an alternative that triggers a fork() system > call. I don't have a strong opinion about which alternative to choose. Do > you perhaps have a preference? The awk option is more readable, and we're not trying to win any performance awards in blktests. Let's just go with that.
diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma index 65ebb7b7f5f7..545a81e8c18e 100644 --- a/common/multipath-over-rdma +++ b/common/multipath-over-rdma @@ -129,19 +129,12 @@ held_by() { done } -# System uptime in seconds. -uptime_s() { - local a b - - echo "$(</proc/uptime)" | { read -r a b && echo "${a%%.*}"; } -} - # Sleep until either $1 seconds have elapsed or until the deadline $2 has been # reached. Return 1 if and only if the deadline has been met. sleep_until() { local duration=$1 deadline=$2 u - u=$(uptime_s) + u=$(_uptime_s) if [ $((u + duration)) -le "$deadline" ]; then sleep "$duration" else diff --git a/common/rc b/common/rc index 41aee3aaa735..c00f2fe1f463 100644 --- a/common/rc +++ b/common/rc @@ -246,3 +246,12 @@ _test_dev_is_partition() { _filter_xfs_io_error() { sed -e 's/^\(.*\)64\(: .*$\)/\1\2/' } + +# System uptime in seconds. +_uptime_s() { + local a b + + echo "$(</proc/uptime)" | { + read -r a b && echo "$b" >/dev/null && echo "${a%%.*}"; + } +} diff --git a/tests/nvmeof-mp/rc b/tests/nvmeof-mp/rc index 2493fcee12de..278843a1270d 100755 --- a/tests/nvmeof-mp/rc +++ b/tests/nvmeof-mp/rc @@ -113,7 +113,7 @@ simulate_network_failure_loop() { [ -e "$dev" ] || return $? [ -n "$duration" ] || return $? - deadline=$(($(uptime_s) + duration)) + deadline=$(($(_uptime_s) + duration)) while [ $rc = 0 ]; do sleep_until 5 ${deadline} || break for d in $(held_by "$dev"); do diff --git a/tests/srp/014 b/tests/srp/014 index 8ecd8a439a82..7afde6284b83 100755 --- a/tests/srp/014 +++ b/tests/srp/014 @@ -69,7 +69,7 @@ sg_reset_loop() { [ -e "$dev" ] || return $? [ -n "$duration" ] || return $? reset_type=(-d -b) - deadline=$(($(uptime_s) + duration)) + deadline=$(($(_uptime_s) + duration)) while true; do sleep_until 1 ${deadline} || break cmd="sg_reset --no-esc ${reset_type[i++ % 2]} $dev" diff --git a/tests/srp/rc b/tests/srp/rc index 696d94e5fb97..a1bc09b496ec 100755 --- a/tests/srp/rc +++ b/tests/srp/rc @@ -247,7 +247,7 @@ simulate_network_failure_loop() { [ -e "$dev" ] || return $? [ -n "$duration" ] || return $? - deadline=$(($(uptime_s) + duration)) + deadline=$(($(_uptime_s) + duration)) s=5 while [ $rc = 0 ]; do sleep_until 5 ${deadline} || break
Make it easy to use the uptime_s() function from block tests. Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- common/multipath-over-rdma | 9 +-------- common/rc | 9 +++++++++ tests/nvmeof-mp/rc | 2 +- tests/srp/014 | 2 +- tests/srp/rc | 2 +- 5 files changed, 13 insertions(+), 11 deletions(-)