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

2007-05-30 Thread Jeff Layton
On Wed, 30 May 2007 10:28:57 +1000
David Chinner [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-05-29 Thread 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 [EMAIL PROTECTED]

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,
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-05-29 Thread 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

-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-05-29 Thread Jeff Layton
On Tue, 29 May 2007 23:38:13 +0400
Alexey Dobriyan [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-05-29 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html