Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: ie1e949a3:Fastmail
Date:   Wed, 12 Apr 2023 12:05:51 +0900
From:   Shin'ichiro Kawasaki <shinichiro@fastmail.com>
To:     Akinobu Mita <akinobu.mita@gmail.com>
Cc:     chaitanyak@nvidia.com, akpm@linux-foundation.org, axboe@kernel.dk,
        hch@infradead.org, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH blktests] don't require modular null_blk for
 fault-injection
Message-ID: <c57uedevavvjwlcvgxfv453dfiy4cgq4f23dscu5dy77sujgw2@pb6fyyfov5se>
References: <7c00f4f8-5bdf-4fa6-d9f0-141cf88c9ec9@nvidia.com>
 <20230329162145.147395-1-akinobu.mita@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230329162145.147395-1-akinobu.mita@gmail.com>
Precedence: bulk

Akinobu, thanks for the patch, and sorry for this slow response.

On Mar 30, 2023 / 01:21, Akinobu Mita wrote:
> I'm trying to allow configuration of null_blk fault-injection via configfs.
> 
> This blktests change changes null_blk fault-injection settings to be
> configured via configfs instead of module parameters.
> This allows null_blk fault-injection tests to run even if the null_blk is
> built-in the kernel and not built as a module.

It's good that the three test case can run with built-in null_blk.

> 
> However, to keep the scripts simple, we will skip testing if the null_blk
> does not yet support configuring fault-injection via configfs.

Hmm, it means that blktests coverage will be lost on older kernels without the
configfs support. Before this change, the test cases were not skipped on older
kernel with loadable null_blk. After this patch, the test cases will be skipped.

Can we cover both the new and old ways? When the null_blk set up with configfs
fails, we can fallback to the old way with module parameters.

For example, null_blk set up of block/014 can be like this:

	# Here, we fail 50% of I/Os.
	if ! _configure_null_blk faultb0 timeout_inject/probability=50 \
	     timeout_inject/times=-1 timeout_inject/verbose=0 power=1 \
	     > /dev/null 2>&1; then
		rmdir /sys/kernel/config/nullb/faultb0
		if [[ -d /sys/kernel/config/nullb/faultb0/timeout_inject ]]; then
			echo "Configuring null_blk failed"
			return 1
		elif _module_file_exists null_blk; then
			# Fall back to set up with module parameter. The format
			# is "<interval>,<probability>,<space>,<times>"
			if ! _init_null_blk timeout='1,50,0,-1'; then
				echo "Configuring null_blk failed"
				return 1;
			fi
		else
			SKIP_REASONS+=("requires fault injection via configfs or modular null_blk")
			return 1
		fi
	fi