Re: [PATCH 35/45] xfs: introduce per-cpu CIL tracking sructure

Dave Chinner
2021-03-11T06:43:28Z

On Thu, Mar 11, 2021 at 05:33:38PM +1100, Dave Chinner wrote:
> On Wed, Mar 10, 2021 at 04:11:43PM -0800, Darrick J. Wong wrote:
> > On Fri, Mar 05, 2021 at 04:11:33PM +1100, Dave Chinner wrote:
> > > From: Dave Chinner <dchinner@redhat.com>
> > >
> > > The CIL push lock is highly contended on larger machines, becoming a
> > > hard bottleneck that about 700,000 transaction commits/s on >16p
> > > machines. To address this, start moving the CIL tracking
> > > infrastructure to utilise per-CPU structures.
> > >
> > > We need to track the space used, the amount of log reservation space
> > > reserved to write the CIL, the log items in the CIL and the busy
> > > extents that need to be completed by the CIL commit. This requires
> > > a couple of per-cpu counters, an unordered per-cpu list and a
> > > globally ordered per-cpu list.
> > >
> > > Create a per-cpu structure to hold these and all the management
> > > interfaces needed, as well as the hooks to handle hotplug CPUs.
> > >
> > > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > > ---
> > > fs/xfs/xfs_log_cil.c | 94 ++++++++++++++++++++++++++++++++++++++
> > > fs/xfs/xfs_log_priv.h | 15 ++++++
> > > include/linux/cpuhotplug.h | 1 +
> > > 3 files changed, 110 insertions(+)
> > >
> > > diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> > > index f8fb2f59e24c..1bcf0d423d30 100644
> > > --- a/fs/xfs/xfs_log_cil.c
> > > +++ b/fs/xfs/xfs_log_cil.c
> > > @@ -1365,6 +1365,93 @@ xfs_log_item_in_current_chkpt(
> > > return true;
> > > }
> > >
> > > +#ifdef CONFIG_HOTPLUG_CPU
> > > +static LIST_HEAD(xlog_cil_pcp_list);
> > > +static DEFINE_SPINLOCK(xlog_cil_pcp_lock);
> > > +static bool xlog_cil_pcp_init;
> > > +
> > > +static int
> > > +xlog_cil_pcp_dead(
> > > + unsigned int cpu)
> > > +{
> > > + struct xfs_cil *cil;
> > > +
> > > + spin_lock(&xlog_cil_pcp_lock);
> > > + list_for_each_entry(cil, &xlog_cil_pcp_list, xc_pcp_list) {
> >
> > Weird indentation.
> >
> > > + /* move stuff on dead CPU to context */
> >
> > Should this have some actual code? I don't think any of the remaining
> > patches add anything here.
>
> They should be moving stuff to the current CIL ctx so it is captured
> when the CPU goes down.

Yup, looks like I missed updating this. Will add it in the patches
that need it.

-Dave.
--
Dave Chinner
david@fromorbit.com