Re: [PATCH blktests v2 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mar 20, 2020 / 14:42, Omar Sandoval wrote:
> On Sun, Mar 15, 2020 at 03:13:20PM -0700, Bart Van Assche wrote:
> > Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
> > Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
> > test only runs if a recently added fault injection feature is available,
> > namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
> > injection").
> > 
> > Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> > Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> > ---
> >  tests/block/030     | 42 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/block/030.out |  1 +
> >  2 files changed, 43 insertions(+)
> >  create mode 100755 tests/block/030
> >  create mode 100644 tests/block/030.out
> > 
> > diff --git a/tests/block/030 b/tests/block/030
> > new file mode 100755
> > index 000000000000..861c85c27f09
> > --- /dev/null
> > +++ b/tests/block/030
> > @@ -0,0 +1,42 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright 2020 Google LLC
> > +#
> > +# Trigger the blk_mq_realloc_hw_ctxs() error path.
> > +
> > +. tests/block/rc
> > +. common/null_blk
> > +
> > +DESCRIPTION="trigger the blk_mq_realloc_hw_ctxs() error path"
> > +QUICK=1
> > +
> > +requires() {
> > +	_have_null_blk || return $?
> > +	_have_module_param null_blk init_hctx || return $?
> > +}
> > +
> > +test() {
> > +	local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
> > +
> > +	: "${TIMEOUT:=30}"
> > +	if ! _init_null_blk nr_devices=0 queue_mode=2 "init_hctx=$(nproc),100,0,0"; then
> > +		echo "Loading null_blk failed"
> > +		return 1
> > +	fi
> > +	if ! _configure_null_blk nullb0 completion_nsec=0 blocksize=512 size=16\
> > +	     submit_queues="$(nproc)" memory_backed=1 power=1; then
> > +		echo "Configuring null_blk failed"
> > +		return 1
> > +	fi
> > +	if { echo "$(<"$sq")" >$sq; } 2>/dev/null; then
> > +		for ((i=0;i<100;i++)); do
> > +			echo 1 >$sq
> > +			nproc >$sq
> > +		done
> > +	else
> > +		echo "Skipping test because $sq cannot be modified" >>"$FULL"
> 
> I just pushed the support for allowing skipping from the middle of a
> test, so now you could make this
> 
> 	SKIP_REASON="Skipping test because $sq cannot be modified"
>

Hi Omar,

I noticed the commit cd11d001fe86 ("Support skipping tests from
test{,_device}()") have side effects to some test cases. The unexpected run
result "not run" is reported to test cases as follows:

- block/005 ... for non-rotational drives such as nullb
- zbd/00[1-5]
- zbd/007   ... for dm-linear device

Some helper functions _test_dev_is_rotational, _test_dev_is_partition or so
are called within test_device() context, and left SKIP_REASON with values. It
resulted in the "not run" result.

To fix this, I can think of two approaches. One is to unset SKIP_REASON after
calling _test_dev_is_X helper functions within the test cases affected. This fix
is straight forward but it will require similar care for future test case
additions and changes. The other approach is to flag the test cases which judge
skip during test run. For example, SKIP_DURING_RUN=1 can be defined in those
test cases (block/030). The check script validates SKIP_REASON only if the test
case is flagged. This second approach looks less costly for me.

Please let me know your thoughts about the fix approach. Thanks!

-- 
Best Regards,
Shin'ichiro Kawasaki



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux