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;
Date:   Wed, 3 May 2023 11:42:26 +0200
From:   Daniel Wagner <dwagner@suse.de>
To:     Chaitanya Kulkarni <chaitanyak@nvidia.com>
Cc:     "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Shin'ichiro Kawasaki <shinichiro@fastmail.com>,
        Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH blktests v3 04/12] common/xfs: Limit fio size job to fit
 into xfs fs
Message-ID: <qv4br55wggqf46jomv7r7ern6e6s7kprqh65wtqvmcnvrlqvl2@kahiiqhulyan>
References: <20230503080258.14525-1-dwagner@suse.de>
 <20230503080258.14525-5-dwagner@suse.de>
 <74e9c631-23cd-705c-7043-88f345598ad1@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <74e9c631-23cd-705c-7043-88f345598ad1@nvidia.com>
Precedence: bulk

On Wed, May 03, 2023 at 09:29:04AM +0000, Chaitanya Kulkarni wrote:
> On 5/3/23 01:02, Daniel Wagner wrote:
> > The usable capacity of the filesystem is less than the raw
> > partition/device size due to the additional meta/log data.
> >
> > Ensure that the job size for fio is not exceeding the limits.
> >
> > Because we have hard coded the path where we mount the filesystem
> > and don't want to expose this, we just update max size inside
> > _xfs_run_fio_verify_io(). No need to leak this into the caller.
> >
> > Signed-off-by: Daniel Wagner <dwagner@suse.de>
> > ---
> >   common/xfs | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/common/xfs b/common/xfs
> > index 413c2820ffaf..37ce85878df2 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -37,6 +37,9 @@ _xfs_run_fio_verify_io() {
> >   		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}/"
> 
> 
> this is exactly how it should to start with, the only
> is now we are silently reducing the fio job size maybe that is okay ?
> 
> or we should error out here instead of being smart ?

The problem I try to solve here is that on the caller side (nvme/035) we don't
know yet the usable filesystem size. We only the size of the nvme_img_size.

If we want to move this logic to the caller side we need split
_xfs_run_fio_verify_io() into steps. The first step which creates the fileystem
and the second one which runs fio.

I don't know if this is worth doing it though.