Re: [PATCH 02/46] sysfs: crash debugging

[Greg KH <greg@kroah.com>]

On Thu, Oct 16, 2008 at 01:40:56PM -0400, Mathieu Desnoyers wrote:
> * Greg Kroah-Hartman (gregkh@suse.de) wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> > 
> > Print the name of the last-accessed sysfs file when we oops, to help track
> > down oopses which occur in sysfs store/read handlers.  Because these oopses
> > tend to not leave any trace of the offending code in the stack traces.
> > 
> > Cc: Kay Sievers <kay.sievers@vrfy.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> > ---
> >  arch/x86/kernel/dumpstack_32.c |    2 ++
> >  arch/x86/kernel/dumpstack_64.c |    2 ++
> >  fs/sysfs/file.c                |   13 +++++++++++++
> >  include/linux/sysfs.h          |    6 ++++++
> >  4 files changed, 23 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/dumpstack_32.c b/arch/x86/kernel/dumpstack_32.c
> > index 201ee35..1a78180 100644
> > --- a/arch/x86/kernel/dumpstack_32.c
> > +++ b/arch/x86/kernel/dumpstack_32.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/kexec.h>
> >  #include <linux/bug.h>
> >  #include <linux/nmi.h>
> > +#include <linux/sysfs.h>
> >  
> >  #include <asm/stacktrace.h>
> >  
> > @@ -343,6 +344,7 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
> >  	printk("DEBUG_PAGEALLOC");
> >  #endif
> >  	printk("\n");
> > +	sysfs_printk_last_file();
> >  	if (notify_die(DIE_OOPS, str, regs, err,
> >  			current->thread.trap_no, SIGSEGV) == NOTIFY_STOP)
> >  		return 1;
> > diff --git a/arch/x86/kernel/dumpstack_64.c b/arch/x86/kernel/dumpstack_64.c
> > index 086cc81..96a5db7 100644
> > --- a/arch/x86/kernel/dumpstack_64.c
> > +++ b/arch/x86/kernel/dumpstack_64.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/kexec.h>
> >  #include <linux/bug.h>
> >  #include <linux/nmi.h>
> > +#include <linux/sysfs.h>
> >  
> >  #include <asm/stacktrace.h>
> >  
> > @@ -489,6 +490,7 @@ int __kprobes __die(const char *str, struct pt_regs *regs, long err)
> >  	printk("DEBUG_PAGEALLOC");
> >  #endif
> >  	printk("\n");
> > +	sysfs_printk_last_file();
> >  	if (notify_die(DIE_OOPS, str, regs, err,
> >  			current->thread.trap_no, SIGSEGV) == NOTIFY_STOP)
> >  		return 1;
> > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> > index c9e4e50..ce8339c 100644
> > --- a/fs/sysfs/file.c
> > +++ b/fs/sysfs/file.c
> > @@ -19,10 +19,18 @@
> >  #include <linux/poll.h>
> >  #include <linux/list.h>
> >  #include <linux/mutex.h>
> > +#include <linux/limits.h>
> >  #include <asm/uaccess.h>
> >  
> >  #include "sysfs.h"
> >  
> > +/* used in crash dumps to help with debugging */
> > +static char last_sysfs_file[PATH_MAX];
> > +void sysfs_printk_last_file(void)
> > +{
> > +	printk(KERN_EMERG "last sysfs file: %s\n", last_sysfs_file);
> > +}
> > +
> >  /*
> >   * There's one sysfs_buffer for each open file and one
> >   * sysfs_open_dirent for each sysfs_dirent with one or more open
> > @@ -328,6 +336,11 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
> >  	struct sysfs_buffer *buffer;
> >  	struct sysfs_ops *ops;
> >  	int error = -EACCES;
> > +	char *p;
> > +
> > +	p = d_path(&file->f_path, last_sysfs_file, sizeof(last_sysfs_file));
> > +	if (p)
> 
> if (!IS_ERR(p)) ?
> 
> > +		memmove(last_sysfs_file, p, strlen(p) + 1);
> >  
> 
> Given we do a strlen() here anyway, shouldn't we just use strcpy ?

Probably would be good.

Care to send a patch for both of these?

thanks,

greg k-h