From: Steven Whitehouse <swhiteho@redhat.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [GFS2 PATCH] GFS2: Log the reason for log flushes in every log header
Date: Mon, 11 Dec 2017 12:12:05 +0000 [thread overview]
Message-ID: <4ef7ef35-a42e-a779-5c9e-e5f411c230e8@redhat.com> (raw)
In-Reply-To: <5adc2737-1f11-27b1-eb46-733fdd9ab9df@redhat.com>
Hi,
On 11/12/17 11:53, Andrew Price wrote:
> On 08/12/17 15:30, Bob Peterson wrote:
>> Hi,
>>
>> This is a version 2 replacement for this patch, with some improvements.
>> ---
>> This patch just adds the capability for GFS2 to track which function
>> called gfs2_log_flush. This should make it easier to diagnose
>> problems based on the sequence of events found in the journals.
>>
>> Signed-off-by: Bob Peterson <rpeterso@redhat.com>
> <snip>
>> --- a/include/uapi/linux/gfs2_ondisk.h
>> +++ b/include/uapi/linux/gfs2_ondisk.h
>> @@ -406,6 +406,42 @@ struct gfs2_log_header {
>> ????? __be32 lh_hash;
>> ? };
>> ? +/*
>> + * Log Header version 2 constants - actor who wrote a log header
>> + */
>> +
>> +enum gfs2_log_flushers {
>> +??? /* Constants reserved for kernel space */
>> +??? LHF_SHUTDOWN??????? = 0,
>
> Should 0 be used for "unknown"?
>
Yes
>> +??? LHF_JDATA_WPAGES??? = 1,
>> +??? LHF_SET_FLAGS??????? = 2,
>> +??? LHF_AIL_EMPTY_GL??? = 3,
>> +??? LHF_AIL_FLUSH??????? = 4,
>> +??? LHF_RGRP_GO_SYNC??? = 5,
>> +??? LHF_INODE_GO_SYNC??? = 6,
>> +??? LHF_INODE_GO_INVAL??? = 7,
>> +??? LHF_FREEZE_GO_SYNC??? = 8,
>> +??? LHF_KILL_SB??????? = 9,
>> +??? LHF_DO_SYNC??????? = 10,
>> +??? LHF_INPLACE_RESERVE??? = 11,
>> +??? LHF_WRITE_INODE??????? = 12,
>> +??? LHF_MAKE_FS_RO??????? = 13,
>> +??? LHF_SYNC_FS??????? = 14,
>> +??? LHF_EVICT_INODE??????? = 15,
>> +??? LHF_TRANS_END??????? = 16,
>> +??? LHF_LOGD_JFLUSH_REQD??? = 17,
>> +??? LHF_LOGD_AIL_FLUSH_REQD??? = 18,
>> +??? LHF_LOG_FLUSHERS??? = 19, /* number of kernel log flushers */
>> +
>> +??? /* Constants reserved for user space / gfs2-utils */
>> +??? LHF_GFS2_CONVERT?????????? = 26,
>> +??? LHF_GFS2_EDIT??????? = 27,
>> +??? LHF_GFS2_FSCK??????? = 28,
>> +??? LHF_GFS2_FSCK_JREPLAY??? = 29,
>> +??? LHF_GFS2_MKFS??????? = 30,
>> +??? LHF_GFS2_JADD??????? = 31
>
> Would it be better to have values for the purposes that the utils
> would need to touch the log header, e.g. LHF_UTIL_REPLAY, LHF_UTIL_FIX
> or LHF_UTIL_CREATE. That way, if a util is renamed or a new util is
> written (even some third party tool that we don't know about) they
> would still have a sensible value to use.
>
> Cheers,
> Andy
>
Yes. Also lets not make this an enum. It would make more sense to have
it as a set of flags. That way we can have a single field but containing
two different meanings. One set would give us the reason for the log
header (normal log flush, recovery, freeze, or whatever) and another
single bit flag telling us whether it is userspace or kernel space. We
can then have an origin, which can be numbered independently in
userspace and kernel space which indicates which function it was called
from.
That also gets rid of that big log_flush_types array. So a typical call
to gfs2_log_flush() would look like:
gfs2_log_flush(sdp, NULL, GFS2_LF_SYNC|GFS2_LO_KILLSB);
in this case assuming it is called from gfs2_kill_sb(). The flag for the
kernel would be added automatically in log_write_header(), since it
would be needed for everything. That would make the code rather easier
to understand, since the two independent fields would be declared
separately. There are 32 bits to use, so no issue to divide it into two
16 bit sub fields for example,
Steve.
next prev parent reply other threads:[~2017-12-11 12:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1917016702.37199765.1512156875142.JavaMail.zimbra@redhat.com>
2017-12-01 19:34 ` [Cluster-devel] [GFS2 PATCH] GFS2: Log the reason for log flushes in every log header Bob Peterson
2017-12-04 12:12 ` Steven Whitehouse
2017-12-08 15:30 ` Bob Peterson
2017-12-11 11:53 ` Andrew Price
2017-12-11 12:12 ` Steven Whitehouse [this message]
2017-12-11 13:23 ` Bob Peterson
2017-12-11 15:32 ` Andrew Price
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=4ef7ef35-a42e-a779-5c9e-e5f411c230e8@redhat.com \
--to=swhiteho@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.