LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: 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: Sun, 1 Mar 2015 08:31:26 +1100	[thread overview]
Message-ID: <20150228213126.GM4251@dastard> (raw)
In-Reply-To: <20150228142557.GB19552@p183.telecom.by>

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

> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -303,9 +303,6 @@ extern char ___assert_task_state[1 - 2*!!(
>  
>  #endif
>  
> -/* Task command name length */
> -#define TASK_COMM_LEN 16
> -
>  #include <linux/spinlock.h>
>  
>  /*
> --- a/include/uapi/linux/sched.h
> +++ b/include/uapi/linux/sched.h
> @@ -49,4 +49,7 @@
>   */
>  #define SCHED_FLAG_RESET_ON_FORK	0x01
>  
> +/* Task command name length */
> +#define TASK_COMM_LEN 16
> +
>  #endif /* _UAPI_LINUX_SCHED_H */

That should be a separate patch, sent to the scheduler maintainers
for review. AFAICT, it isn't part of the user API - it's not defined
in the man page which just says "can be up to 16 bytes".

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2015-02-28 21:31 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 [this message]
2015-03-02  4:38                 ` Mateusz Guzik
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=20150228213126.GM4251@dastard \
    --to=david@fromorbit.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cluster-devel@redhat.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).