From: Bob Peterson <rpeterso@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH] gfs2: Don't free rgrp clone bitmaps until go_inval
Date: Thu, 11 May 2023 10:18:45 -0400 [thread overview]
Message-ID: <bc766d93-8000-5736-64e5-16df4a4a8136@redhat.com> (raw)
In-Reply-To: <d718cce68afe7db827103dbc4886f4982b1ad83a.camel@redhat.com>
Hi,
On 5/11/23 4:47 AM, Steven Whitehouse wrote:
> Hi,
>
>> This repeated kmalloc -> kfree -> kmalloc -> kfree is a waste of
>> time:
>
> It is a waste of time. However, if the clones are kept around for lots
> of rgrps, then that is a waste of space. The question is really what
> the correct balance is.
>
> Can we not solve the problem at source and prevent the large number of
> lock transitions referred to above? If not then it might be a good plan
> to document why that is the case,
>
> Steve.
I do believe from a performance perspective, we might benefit just as
much (if not more) by, for example, using the LM_FLAG_ANY flag when
acquiring rgrp glocks in SH mode such as gfs2_check_blk_type.
That way if the lock is already in EX, we can just use it rather than
demote and promote it. There may be other places in the code where we
can do the same.
From a correctness perspective, one of my concerns is:
The go_inval() code is only run when transitioning to UN or DF, so
transitions from EX to SH and back won't call go_inval. They always call
go_sync, but rgrp_go_sync code exits (and avoids the whole bitmap issue)
unless the glock is GLF_DIRTY. It just seems fraught with pitfalls. But
still, it seems to work properly.
I had a meeting with Andreas this morning and we decided that since it
seems to work, "if it's not broken, don't fix it." So for now, I retract
the patch and we can readdress the issue if we find problems related to it.
Regards,
Bob Peterson
next prev parent reply other threads:[~2023-05-11 14:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-10 19:08 [Cluster-devel] [PATCH] gfs2: Don't free rgrp clone bitmaps until go_inval Bob Peterson
2023-05-11 8:47 ` Steven Whitehouse
2023-05-11 14:18 ` Bob Peterson [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-04-25 16:25 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=bc766d93-8000-5736-64e5-16df4a4a8136@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 read-only IMAP folder(s) and NNTP newsgroup(s).