Re: [PATCH] add procfs tunable to enable immediate panic when there are busy inodes after umount
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
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
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
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
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