From: "Darrick J. Wong" <djwong@kernel.org>
To: Zorro Lang <zlang@redhat.com>
Cc: linux-xfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH v2.1 1/2] check: detect and preserve all coredumps made by a test
Date: Fri, 14 Oct 2022 11:15:24 -0700 [thread overview]
Message-ID: <Y0mnPD3bJhmo0S0K@magnolia> (raw)
In-Reply-To: <Y0g8XPzJHw6eICoK@magnolia>
On Thu, Oct 13, 2022 at 09:27:08AM -0700, Darrick J. Wong wrote:
> On Fri, Oct 14, 2022 at 12:03:26AM +0800, Zorro Lang wrote:
> > On Thu, Oct 13, 2022 at 08:48:41AM -0700, Darrick J. Wong wrote:
> > > On Thu, Oct 13, 2022 at 07:44:46PM +0800, Zorro Lang wrote:
> > > > On Wed, Oct 12, 2022 at 05:19:50PM -0700, Darrick J. Wong wrote:
> > > > > From: Darrick J. Wong <djwong@kernel.org>
> > > > >
> > > > > If someone sets kernel.core_uses_pid (or kernel.core_pattern), any
> > > > > coredumps generated by fstests might have names that are longer than
> > > > > just "core". Since the pid isn't all that useful by itself, let's
> > > > > record the coredumps by hash when we save them, so that we don't waste
> > > > > space storing identical crash dumps.
> > > > >
> > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > > > ---
> > > > > v2.1: use REPORT_DIR per maintainer suggestion
> > > > > ---
> > > >
> > > > This version looks good to me,
> > > > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > >
> > > It occurred to me overnight that ./check doesn't export REPORT_DIR, so
> > > I'll push out a v2.2 that adds that. Currently the lack of an export
> > > doesn't affect anyone, but as soon as any tests want to call
> > > _save_coredump they're going to run into that issue.
> >
> > Hmm... the RESULT_DIR is exported, you can use it, or use $seqres directly due
> > to it's defined in common/preamble (although is not exported).
> >
> > ./common/preamble:42: seqres=$RESULT_DIR/$seq
> >
> > What do you think?
>
> seqres is defined differently in check than in common/preamble, but I
> guess RESULT_DIR will work.
Nope, it won't, because RESULT_DIR ends up getting set to something
like /var/tmp/fstests/xfs_moocow/xfs, seqnum gets set to xfs/350, and
then you end up with garbage paths like:
/var/tmp/fstests/xfs_moocow/xfs/xfs/350.core.XXX
Soooo you were right, I should have used seqres from the start. The
multiple definitions are confusing, but they end up resolving to the
same pathnames(!) so it's all good.
--D
> --D
>
> > Thanks,
> > Zorro
> >
> > >
> > > (...and yes, I do have future fuzz tests that will call it from a test
> > > in between fuzz field cycles.)
> > >
> > > --D
> > >
> > > > > check | 26 ++++++++++++++++++++++----
> > > > > common/rc | 16 ++++++++++++++++
> > > > > 2 files changed, 38 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/check b/check
> > > > > index d587a70546..29303db1c8 100755
> > > > > --- a/check
> > > > > +++ b/check
> > > > > @@ -923,11 +923,19 @@ function run_section()
> > > > > sts=$?
> > > > > fi
> > > > >
> > > > > - if [ -f core ]; then
> > > > > - _dump_err_cont "[dumped core]"
> > > > > - mv core $RESULT_BASE/$seqnum.core
> > > > > + # If someone sets kernel.core_pattern or kernel.core_uses_pid,
> > > > > + # coredumps generated by fstests might have a longer name than
> > > > > + # just "core". Use globbing to find the most common patterns,
> > > > > + # assuming there are no other coredump capture packages set up.
> > > > > + local cores=0
> > > > > + for i in core core.*; do
> > > > > + test -f "$i" || continue
> > > > > + if ((cores++ == 0)); then
> > > > > + _dump_err_cont "[dumped core]"
> > > > > + fi
> > > > > + _save_coredump "$i"
> > > > > tc_status="fail"
> > > > > - fi
> > > > > + done
> > > > >
> > > > > if [ -f $seqres.notrun ]; then
> > > > > $timestamp && _timestamp
> > > > > @@ -960,6 +968,16 @@ function run_section()
> > > > > # of the check script itself.
> > > > > (_adjust_oom_score 250; _check_filesystems) || tc_status="fail"
> > > > > _check_dmesg || tc_status="fail"
> > > > > +
> > > > > + # Save any coredumps from the post-test fs checks
> > > > > + for i in core core.*; do
> > > > > + test -f "$i" || continue
> > > > > + if ((cores++ == 0)); then
> > > > > + _dump_err_cont "[dumped core]"
> > > > > + fi
> > > > > + _save_coredump "$i"
> > > > > + tc_status="fail"
> > > > > + done
> > > > > fi
> > > > >
> > > > > # Reload the module after each test to check for leaks or
> > > > > diff --git a/common/rc b/common/rc
> > > > > index d877ac77a0..2e1891180a 100644
> > > > > --- a/common/rc
> > > > > +++ b/common/rc
> > > > > @@ -4949,6 +4949,22 @@ _create_file_sized()
> > > > > return $ret
> > > > > }
> > > > >
> > > > > +_save_coredump()
> > > > > +{
> > > > > + local path="$1"
> > > > > +
> > > > > + local core_hash="$(_md5_checksum "$path")"
> > > > > + local out_file="$REPORT_DIR/$seqnum.core.$core_hash"
> > > > > +
> > > > > + if [ -s "$out_file" ]; then
> > > > > + rm -f "$path"
> > > > > + return
> > > > > + fi
> > > > > + rm -f "$out_file"
> > > > > +
> > > > > + mv "$path" "$out_file"
> > > > > +}
> > > > > +
> > > > > init_rc
> > > > >
> > > > > ################################################################################
> > > > >
> > > >
> > >
> >
next prev parent reply other threads:[~2022-10-14 18:15 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-12 1:45 [PATCHSET v2 0/2] fstests: improve coredump capture and storage Darrick J. Wong
2022-10-12 1:45 ` [PATCH 1/2] check: detect and preserve all coredumps made by a test Darrick J. Wong
2022-10-12 15:47 ` Zorro Lang
2022-10-12 15:51 ` Darrick J. Wong
2022-10-13 0:19 ` [PATCH v2.1 " Darrick J. Wong
2022-10-13 11:44 ` Zorro Lang
2022-10-13 15:48 ` Darrick J. Wong
2022-10-13 16:03 ` Zorro Lang
2022-10-13 16:27 ` Darrick J. Wong
2022-10-14 18:15 ` Darrick J. Wong [this message]
2022-10-14 18:20 ` [PATCH v2.2 " Darrick J. Wong
2022-10-12 1:45 ` [PATCH 2/2] check: optionally compress core dumps Darrick J. Wong
2022-10-13 11:51 ` Zorro Lang
2022-10-13 15:50 ` Darrick J. Wong
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=Y0mnPD3bJhmo0S0K@magnolia \
--to=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=zlang@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).