Re: [PATCH v3] fs: record task name which froze superblock

[Mateusz Guzik]

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@xxxxxxxxx>
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/