cluster-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 09/11] gfs2: Read bitmap buffers on demand
Date: Mon, 14 Jan 2019 12:12:10 -0500 (EST)	[thread overview]
Message-ID: <1059840510.64229774.1547485930332.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <20190111160548.9423-10-agruenba@redhat.com>

----- 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 <agruenba@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.

Regards,

Bob Peterson



  reply	other threads:[~2019-01-14 17:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-11 16:05 [Cluster-devel] [PATCH 00/11] Read bitmap buffers on demand Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 01/11] gfs2: Fix setting GBF_FULL in gfs2_rbm_find Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 02/11] gfs2: Rename minext => minlen Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 03/11] gfs2: Don't use struct gfs2_rbm in struct gfs2_extent Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 04/11] gfs2: Minor gfs2_free_extlen cleanup Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 05/11] gfs2: Only use struct gfs2_rbm for bitmap manipulations Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 06/11] gfs2: Minor gfs2_adjust_reservation and gfs2_alloc_extent cleanups Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 07/11] gfs2: Minor gfs2_rgrp_send_discards cleanup Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 08/11] gfs2: Add buffer head to struct gfs2_rgrpd Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 09/11] gfs2: Read bitmap buffers on demand Andreas Gruenbacher
2019-01-14 17:12   ` Bob Peterson [this message]
2019-01-14 18:41     ` Andreas Gruenbacher
2019-01-14 22:08       ` Andreas Gruenbacher
2019-01-11 16:05 ` [Cluster-devel] [PATCH 10/11] gfs2: Clean up assertion, consistency check, and error reporting functions Andreas Gruenbacher
2019-01-14 16:27   ` Bob Peterson
2019-01-11 16:05 ` [Cluster-devel] [PATCH 11/11] gfs2: Skip gfs2_metatype_check for cached buffers Andreas Gruenbacher
2019-01-14 16:40   ` Bob Peterson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1059840510.64229774.1547485930332.JavaMail.zimbra@redhat.com \
    --to=rpeterso@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).