LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mateusz Guzik <mguzik@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>, Jan Kara <jack@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	swhiteho@redhat.com, cluster-devel@redhat.com
Subject: Re: [PATCH v3] fs: record task name which froze superblock
Date: Mon, 2 Mar 2015 05:38:29 +0100	[thread overview]
Message-ID: <20150302043828.GA2516@mguzik> (raw)
In-Reply-To: <20150228213126.GM4251@dastard>

On Sun, Mar 01, 2015 at 08:31:26AM +1100, Dave Chinner wrote:
> On Sat, Feb 28, 2015 at 05:25:57PM +0300, Alexey Dobriyan wrote:
> > Freezing and thawing are separate system calls, task which is supposed
> > to thaw filesystem/superblock can disappear due to crash or not thaw
> > due to a bug. At least record task name (we can't take task_struct
> > reference) to make support engineer's life easier.
> > 
> > Hopefully 16 bytes per superblock isn't much.
> > 
> > TASK_COMM_LEN definition (which is userspace ABI, see prctl(PR_SET_NAME)) is
> > moved to userspace exported header to not drag sched.h into every fs.h inclusion.
> > 
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> 
> Freeze/thaw can be nested at the block level. That means the
> sb->s_writers.freeze_comm can point at the wrong process. i.e.
> 
> Task A			Task B
> freeze_bdev
>   freeze_super
>     freeze_comm = A
> 			freeze_bdev
> .....
> thaw_bdev
>  <device still frozen>
> 			<crash>
> 
> At this point, the block device will never be unthawed, but
> the debug field is now pointing to the wrong task. i.e. The debug
> helper has not recorded the process that is actually causing the
> problem, and leads us all off on a wild goose chase down the wrong
> path.
> 
> IMO, debug code is only useful if it's reliable.....
> 

It can be trivially modified to be very useful to support people.

Actually this patch clears saved task name on unfreeze, so in this
particular scenario we would end up with no data.

Freezer and unfreezer names don't even have to match, so there is not
much we can do here (e.g. recording all names in a linked list or
something is a non-starter because of this).

I propose the following:
- on freezing:
1. if 0->1 save the name
2. if 1->2 have a flag to note there is an additional freezer
- on unfreezing
1. if 1->0 clear the flag
2. DO NOT clear the name in any case

This way we keep the name for possible future reference and we know
whether something with this name was the sole freezer in this cycle.

As explained below, this one task name is already very useful and likely
covers majority of real life use cases.

While working in support we were getting a lot of vmcores where hung task
detector panicked the kernel because a lot of tasks were blocked
in UN state trying to write to frozen filesystems. I presume OP has
similar story.

Some back on forth commuication almost always revealed one process e.g.
freezing stuff and then blocking itself trying to access it. While we
could see it blocked, we had no presumptive evidence to pin freezing on
it. A matching name, while still not 100% conclusive, would be ok enough
to push the case forward and avoid a rountrip of systemap scripts
showing freezer process tree.

-- 
Mateusz Guzik

  reply	other threads:[~2015-03-02  4:38 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-14 18:55 [PATCH] " Alexey Dobriyan
2015-02-16  9:38 ` Jan Kara
2015-02-18  7:34   ` Alexey Dobriyan
2015-02-18  7:36     ` [PATCH v2] " Alexey Dobriyan
2015-02-18  9:13     ` [PATCH] " Jan Kara
2015-02-18 10:18       ` Steven Whitehouse
2015-02-20 11:42       ` Alexey Dobriyan
2015-02-20 12:15         ` Jan Kara
2015-02-28 14:22           ` Alexey Dobriyan
2015-02-28 14:25             ` [PATCH v3] " Alexey Dobriyan
2015-02-28 21:31               ` Dave Chinner
2015-03-02  4:38                 ` Mateusz Guzik [this message]
2015-03-02  4:46                   ` Mateusz Guzik
2015-03-09 15:14                     ` Alexey Dobriyan
2015-03-02 21:33                   ` Dave Chinner
2015-03-04 15:14                   ` Alexey Dobriyan

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=20150302043828.GA2516@mguzik \
    --to=mguzik@redhat.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cluster-devel@redhat.com \
    --cc=david@fromorbit.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=swhiteho@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH v3] fs: record task name which froze superblock' \
    /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

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).