[PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[Jeff Layton]

After spending quite a bit of time tracking down a "VFS: busy inodes
after unmount" problem, it occurs to me that it would be nice to be
able to force a panic when that occurs. While an oops message alone is
not generally helpful for tracking down this sort of problem,
collecting and analyzing a coredump when this occurs can be.

The following patch adds a procfs tunable that allows you to force a
core when a "busy inodes after umount" problem occurs. It also changes
the classic error message to be something a bit less cryptic to users.

Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/fs/block_dev.c b/fs/block_dev.c
diff --git a/fs/inode.c b/fs/inode.c
index 9a012cc..0e638b0 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -327,7 +327,7 @@ static int invalidate_list(struct list_head *head, struct list_head *dispose)
 			count++;
 			continue;
 		}
-		busy = 1;
+		++busy;
 	}
 	/* only unused inodes may be cached with i_count zero */
 	inodes_stat.nr_unused -= count;
diff --git a/fs/super.c b/fs/super.c
index 5260d62..9c2871b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -287,6 +287,8 @@ int fsync_super(struct super_block *sb)
 void generic_shutdown_super(struct super_block *sb)
 {
 	const struct super_operations *sop = sb->s_op;
+	extern int umount_debug;
+	int busy;
 
 	if (sb->s_root) {
 		shrink_dcache_for_umount(sb);
@@ -303,10 +305,15 @@ void generic_shutdown_super(struct super_block *sb)
 			sop->put_super(sb);
 
 		/* Forget any remaining inodes */
-		if (invalidate_inodes(sb)) {
-			printk("VFS: Busy inodes after unmount of %s. "
-			   "Self-destruct in 5 seconds.  Have a nice day...\n",
-			   sb->s_id);
+		if (busy = invalidate_inodes(sb)) {
+			printk("VFS: %d busy inodes after unmount of %s. ",
+				 busy, sb->s_id);
+			if (umount_debug != 0) {
+				printk("Crashing host on request.\n");
+				BUG();
+			} else {
+				printk("This machine will likely crash eventually. Consider a reboot.\n");
+			}
 		}
 
 		unlock_kernel();
diff --git a/include/linux/fs.h b/include/linux/fs.h
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 47f1c53..176b984 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -818,6 +818,7 @@ enum
 	FS_AIO_NR=18,	/* current system-wide number of aio requests */
 	FS_AIO_MAX_NR=19,	/* system-wide maximum number of aio requests */
 	FS_INOTIFY=20,	/* inotify submenu */
+	FS_UMOUNT_DEBUG=21, /* busy inodes on umount debug switch */
 	FS_OCFS2=988,	/* ocfs2 */
 };
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 30ee462..8e62c34 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -156,6 +156,7 @@ extern ctl_table pty_table[];
 extern ctl_table inotify_table[];
 #endif
 
+int umount_debug;
 #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
 int sysctl_legacy_va_layout;
 #endif
@@ -962,6 +963,14 @@ static ctl_table fs_table[] = {
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+	{
+		.ctl_name	= FS_UMOUNT_DEBUG,
+		.procname	= "umount_debug",
+		.data		= &umount_debug,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+	},
 #ifdef CONFIG_DNOTIFY
 	{
 		.ctl_name	= FS_DIR_NOTIFY,

Re: [PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[Alexey Dobriyan]

On Tue, May 29, 2007 at 11:40:42AM -0400, Jeff Layton wrote:
> After spending quite a bit of time tracking down a "VFS: busy inodes
> after unmount" problem, it occurs to me that it would be nice to be
> able to force a panic when that occurs. While an oops message alone is
> not generally helpful for tracking down this sort of problem,
> collecting and analyzing a coredump when this occurs can be.
>
> The following patch adds a procfs tunable that allows you to force a
> core when a "busy inodes after umount" problem occurs. It also changes
> the classic error message to be something a bit less cryptic to users.

> @@ -303,10 +305,15 @@ void generic_shutdown_super(struct super_block *sb)
>  			sop->put_super(sb);
>
>  		/* Forget any remaining inodes */
> -		if (invalidate_inodes(sb)) {
> -			printk("VFS: Busy inodes after unmount of %s. "
> -			   "Self-destruct in 5 seconds.  Have a nice day...\n",
> -			   sb->s_id);
> +		if (busy = invalidate_inodes(sb)) {
> +			printk("VFS: %d busy inodes after unmount of %s. ",
> +				 busy, sb->s_id);
> +			if (umount_debug != 0) {
> +				printk("Crashing host on request.\n");
> +				BUG();
> +			} else {
> +				printk("This machine will likely crash eventually. Consider a reboot.\n");
> +			}

You can add just BUG_ON here and do

	echo 1 >/proc/sys/kernel/panic_on_oops

Re: [PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[Jeff Layton]

On Tue, 29 May 2007 23:38:13 +0400
Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Tue, May 29, 2007 at 11:40:42AM -0400, Jeff Layton wrote:
> > After spending quite a bit of time tracking down a "VFS: busy inodes
> > after unmount" problem, it occurs to me that it would be nice to be
> > able to force a panic when that occurs. While an oops message alone is
> > not generally helpful for tracking down this sort of problem,
> > collecting and analyzing a coredump when this occurs can be.
> >
> > The following patch adds a procfs tunable that allows you to force a
> > core when a "busy inodes after umount" problem occurs. It also changes
> > the classic error message to be something a bit less cryptic to users.
> 
> > @@ -303,10 +305,15 @@ void generic_shutdown_super(struct super_block *sb)
> >  			sop->put_super(sb);
> >
> >  		/* Forget any remaining inodes */
> > -		if (invalidate_inodes(sb)) {
> > -			printk("VFS: Busy inodes after unmount of %s. "
> > -			   "Self-destruct in 5 seconds.  Have a nice day...\n",
> > -			   sb->s_id);
> > +		if (busy = invalidate_inodes(sb)) {
> > +			printk("VFS: %d busy inodes after unmount of %s. ",
> > +				 busy, sb->s_id);
> > +			if (umount_debug != 0) {
> > +				printk("Crashing host on request.\n");
> > +				BUG();
> > +			} else {
> > +				printk("This machine will likely crash eventually. Consider a reboot.\n");
> > +			}
> 
> You can add just BUG_ON here and do
> 
> 	echo 1 >/proc/sys/kernel/panic_on_oops
> 

I certainly could, but the problem is that there's little point in
panicing immediately here if you can't collect a coredump. Oops
messages aren't very helpful for tracking this sort of thing down, so
I'd think we want the BUG() conditional on something more granular
than panic_on_oops.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[David Chinner]

On Tue, May 29, 2007 at 11:40:42AM -0400, Jeff Layton wrote:
> After spending quite a bit of time tracking down a "VFS: busy inodes
> after unmount" problem, it occurs to me that it would be nice to be
> able to force a panic when that occurs. While an oops message alone is
> not generally helpful for tracking down this sort of problem,
> collecting and analyzing a coredump when this occurs can be.

Agreed - we've found that we've had roughly 50% success in finding
the cause of these problems from crash dumps triggered immediately
like this vs ~0% from a crash that occurred some time later.

Given that this problem will always result in a crash of the kernel
at some random time in the future, why don't we just make this error
an unconditional panic on get the crash over and done with?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount

[Jeff Layton]

On Wed, 30 May 2007 10:28:57 +1000
David Chinner <dgc@sgi.com> wrote:

> On Tue, May 29, 2007 at 11:40:42AM -0400, Jeff Layton wrote:
> > After spending quite a bit of time tracking down a "VFS: busy inodes
> > after unmount" problem, it occurs to me that it would be nice to be
> > able to force a panic when that occurs. While an oops message alone is
> > not generally helpful for tracking down this sort of problem,
> > collecting and analyzing a coredump when this occurs can be.
> 
> Agreed - we've found that we've had roughly 50% success in finding
> the cause of these problems from crash dumps triggered immediately
> like this vs ~0% from a crash that occurred some time later.
> 
> Given that this problem will always result in a crash of the kernel
> at some random time in the future, why don't we just make this error
> an unconditional panic on get the crash over and done with?
> 

Perhaps that's the best course of action. Then again, there can be a
long time between the problem and crash (weeks even). For someone who
can't collect a coredump, it might be preferable to not immediately
crash the box and allow them to try to reboot it at a convenient time.
That was my reasoning for adding the procfs tunable.

Either way, if the machine doesn't crash immediately, I'd like to see a
different error message here. The current one is confusing to users.
They see it and figure "my box didn't crash in 5 mins, so everything
must be OK!"

-- 
Jeff Layton <jlayton@redhat.com>