On Mon, 14 Jan 2019 at 19:41, Andreas Gruenbacher <agrue...@redhat.com> wrote:
>
> Bob,
>
> On Mon, 14 Jan 2019 at 18:12, Bob Peterson <rpete...@redhat.com> wrote:
> > ----- Original Message -----
> > > Before this patch, when locking a resource group, gfs2 would read in the
> > > resource group header and all the bitmap buffers of the resource group.
> > > Those buffers would then be locked into memory until the resource group
> > > is unlocked, which will happen when the filesystem is unmounted or when
> > > transferring the resource group lock to another node, but not due to
> > > memory pressure.  Larger resource groups lock more buffers into memory,
> > > and cause more unnecessary I/O when resource group locks are transferred
> > > between nodes.
> > >
> > > With this patch, when locking a resource group, only the resource group
> > > header is read in.  The other bitmap buffers (the resource group header
> > > contains part of the bitmap) are only read in on demand.
> > >
> > > It would probably make sense to also only read in the resource group
> > > header on demand, when the resource group is modified, but since the
> > > header contains the number of free blocks in the resource group, there
> > > is a higher incentive to keep the header locked in memory after that.
> > >
> > > Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com>
> > > ---
> >
> > Hi,
> >
> > This patch looks good. However, I'm concerned about its performance,
> > at least until we get some bitmap read-ahead in place.
> > (see "/* FIXME: Might want to do read-ahead here. */")
> >
> > In particular, I'm concerned that it does the exact opposite of this
> > performance enhancement:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/fs/gfs2?id=39b0f1e9290880a6c905f639e7db6b646e302a4f
> >
> > So we need to test that 2GB rgrps (33 bitmap blocks) that are fragmented
> > don't have a disproportionate slowdown compared to, say, 128MB rgrps
> > because we end up spending so much time fetching pages out of page cache,
> > only to find them inadequate and doing brelse() again. And by fragmented,
> > I mean all 33 bitmap blocks have a small number of free blocks, thus
> > disqualifying them from having "full" status but still having a large
> > enough number of free blocks that we might consider searching them.
> > Some of the patches we did after that one may have mitigated the problem
> > to some extent, but we need to be sure we're not regressing performance.
>
> Yes, there are a few ways this patch can still be tuned depending on
> the performance impact. This should speed up "normal" workloads by
> requiring fewer reads from disk when acquiring a resource group glock,
> and less memory is pinned for filesystem metadata, but we do end up
> with more page cache lookups during allocations, and depending on
> read-ahead, possibly more smaller reads at inconvenient times.
>
> rd_last_alloc should help limit the performance impact when bitmaps
> become mostly full. (Forms of rd_last_alloc have been in gfs2 since
> the mainline merge.)

Hmm, rd_extfail_pt actually, not rd_last_alloc, and that predates the
optimization in commit 39b0f1e929 by less than two years.

> One way to further reduce the number of page cache lookups would be to
> cache the buffer head of the last allocation, but it's unclear if this
> is even an issue, so let's do some performance testing first.

Andreas

Reply via email to