Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sun, 15 Aug 2021 09:40:35 +1000
From:   Dave Chinner <dchinner@redhat.com>
To:     Hillf Danton <hdanton@sina.com>
Cc:     kernel test robot <oliver.sang@intel.com>,
        "Darrick J. Wong" <djwong@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, lkp@intel.com,
        zhengjun.xing@linux.intel.com
Subject: Re: [xfs]  6df693ed7b:  aim7.jobs-per-min -15.7% regression
Message-ID: <20210814234035.GE2959@rh>
References: <20210809064248.GB5761@xsang-OptiPlex-9020>
 <20210809093114.3179-1-hdanton@sina.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210809093114.3179-1-hdanton@sina.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Precedence: bulk

On Mon, Aug 09, 2021 at 05:31:14PM +0800, Hillf Danton wrote:
> On Mon, 9 Aug 2021 14:42:48 +0800
> > 
> > FYI, we noticed a -15.7% regression of aim7.jobs-per-min due to commit:
> > 
> > 
> > commit: 6df693ed7ba9ec03cafc38d5064de376a11243e2 ("xfs: per-cpu deferred inode inactivation queues")
> > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git xfs-5.15-merge
> > 
> > 
> > in testcase: aim7
> > on test machine: 88 threads 2 sockets Intel(R) Xeon(R) Gold 6238M CPU @ 2.10GHz with 128G memory
> > with following parameters:
> > 
> > 	disk: 4BRD_12G
> > 	md: RAID1
> > 	fs: xfs
> > 	test: disk_wrt
> > 	load: 3000
> > 	cpufreq_governor: performance
> > 	ucode: 0x5003006
> > 
> 
> See if scheduling can help assuming a bound worker should run as short as
> it could.
> 
> The change below is
> 1/ add schedule entry in inodegc worker, and as compensation allow it to
> repeat gc until no more c available.

Do you have any evidence that this is a problem?

I mean, we bound queue depth to 256 items, and my direct
measurements of workloads show that and typical inactivation
processing does not block and takes roughly 50-100us per item. On
inodes that require lots of work (maybe minutes!), we end up
sleeping on locks or resource reservations fairly quickly, hence we
don't tend to rack up significant amount of uninterrupted CPU time
in this loop at all.

> 2/ make inodegc_wq unbound to spawn workers because they are no longer
> potential CPU hogs (and this is not mandatory but optional).
> 
> to see if hot cache outweights spawning of workers.

NACK. We already know what impact that has:  moving to bound
workqueues erased a 50-60% performance degradation in the original
queueing mechanism that used unbound workqueues and required
inactivation to run on cold caches. IOWs, performance analysis lead
us to short bound depth per-cpu queues and single depth bound
per-cpu workqueues. We don't do complex stuff like this unless it is
necessary for performance and scalability...

Cheers,

Dave.
-- 
Dave Chinner
dchinner@redhat.com