lkml.org 
[lkml]   [2011]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/1] sysfs: add more info to the oops dump
On Thu, Mar 10, 2011 at 05:53:08PM +0200, Phil Carmody wrote:
> If we're going to remember which file we accessed, we might as well
> also remember whether it was a read or a write, and if the latter,
> some indication of what was written.
>
> e.g.
> $ echo 1 > /sys/kernel/slab/:at-0000064/sanity_checks
> $ echo c > /proc/sysrq-trigger
> ...
> [ 112.457580] last sysfs file (w): /sys/kernel/slab/:at-0000064/sanity_checks
> [ 112.464569] written: 1

Has this actually helped you out? I've been thinking of removing this
line in the dmesg entirely as I haven't seen it help in a very long time
to track anything down.

>
> Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
> ---
> fs/sysfs/file.c | 21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index da3fefe..281e4dd 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -26,9 +26,18 @@
>
> /* used in crash dumps to help with debugging */
> static char last_sysfs_file[PATH_MAX];
> +static char last_sysfs_write[16];
> +static int last_sysfs_write_len; /* magic value < 0 => read */
> void sysfs_printk_last_file(void)
> {
> - printk(KERN_EMERG "last sysfs file: %s\n", last_sysfs_file);
> + printk(KERN_EMERG "last sysfs file (%c): %s\n",
> + (last_sysfs_write_len < 0) ? 'r' : 'w',
> + last_sysfs_file);
> + if (last_sysfs_write_len >= sizeof(last_sysfs_write))
> + printk(KERN_EMERG " written: %s...(%d chars)\n",
> + last_sysfs_write, last_sysfs_write_len);
> + else if (last_sysfs_write_len > 0)
> + printk(KERN_EMERG " written: %s\n", last_sysfs_write);
> }
>
> /*
> @@ -200,12 +209,19 @@ flush_write_buffer(struct dentry * dentry, struct sysfs_buffer * buffer, size_t
> struct sysfs_dirent *attr_sd = dentry->d_fsdata;
> struct kobject *kobj = attr_sd->s_parent->s_dir.kobj;
> const struct sysfs_ops * ops = buffer->ops;
> + int copylen = min(count, sizeof(last_sysfs_write) - 1);
> int rc;
>
> /* need attr_sd for attr and ops, its parent for kobj */
> if (!sysfs_get_active(attr_sd))
> return -ENODEV;
>
> + while (copylen > 0 && buffer->page[copylen-1] == '\n')
> + --copylen; /* never print trailing \n's */
> + memcpy(last_sysfs_write, buffer->page, copylen);
> + last_sysfs_write[copylen] = '\0';
> + last_sysfs_write_len = count;
> +
> rc = ops->store(kobj, attr_sd->s_attr.attr, buffer->page, count);
>
> sysfs_put_active(attr_sd);
> @@ -363,6 +379,9 @@ static int sysfs_open_file(struct inode *inode, struct file *file)
> if (file->f_mode & FMODE_WRITE) {
> if (!(inode->i_mode & S_IWUGO) || !ops->store)
> goto err_out;
> + last_sysfs_write_len = 0;
> + } else {
> + last_sysfs_write_len = -1; /* magic value */

"magic" values are bad, please use a #define or something else that
makes more sense to those looking at the code in 5 years.

thanks,

greg k-h


\
 
 \ /
  Last update: 2011-03-10 18:07    [W:0.038 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site