| Message ID | 1399464274-16310-1-git-send-email-lczerner@redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 07, 2014 at 02:04:34PM +0200, Lukas Czerner wrote: > > cat /sys/fs/ext4/sda/errors > > If the file system is not marked as containing errors then the file > returns just 0. Otherwise it would print out the following information: > > <error count> first <first_error_time> <first_error_func>:<first_error_line> \ > last <last_error_time> <last_error_func>:<last_error_line> This goes against the typical way in which information is returned in sysfs. Personally, I've always preferred the scheme used by, for example /proc/acpi/battery/BAT0/info, versus needing to read N different files in /sys/class/power_supply/BAT0/*, but the argument is that it's easier for programs to parse information if they are in separate files. It's one of the reasons why I've kept /proc/fs/ext4/sda3/mb_groups, since trying to convert that file over to the Church of Sysfs's style guidelines was far more work than it was worth. I'm not actually sure it's that important to be able to expose the error function and error line number via sysfs or procfs. If a process wants a complete record of all of the various errors, then dmesg or maybe some netlink socket is really the best interface for getting this information. For sysfs, I suspect the primary use will be answering the questions: "is this file system healthy or not", and "when did it first become unhealthy". And for questoins like this, the errors_count and first_error_time and last_error_time is probably the most useful bits of information to expose. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 7 May 2014, Theodore Ts'o wrote: > Date: Wed, 7 May 2014 10:36:46 -0400 > From: Theodore Ts'o <tytso@mit.edu> > To: Lukas Czerner <lczerner@redhat.com> > Cc: linux-ext4@vger.kernel.org > Subject: Re: [PATCH v2] ext4: add sysfs entry showing whether the fs contains > errors > > On Wed, May 07, 2014 at 02:04:34PM +0200, Lukas Czerner wrote: > > > > cat /sys/fs/ext4/sda/errors > > > > If the file system is not marked as containing errors then the file > > returns just 0. Otherwise it would print out the following information: > > > > <error count> first <first_error_time> <first_error_func>:<first_error_line> \ > > last <last_error_time> <last_error_func>:<last_error_line> > > This goes against the typical way in which information is returned in > sysfs. Personally, I've always preferred the scheme used by, for > example /proc/acpi/battery/BAT0/info, versus needing to read N > different files in /sys/class/power_supply/BAT0/*, but the argument is > that it's easier for programs to parse information if they are in > separate files. What about /sys/class/power_supply/BAT0/uevent ? It it is easily parsable and has all the information in /sys/class/power_supply/BAT0/* Also something like /sys/block/sda/stat seems to differ from the rest. > > It's one of the reasons why I've kept /proc/fs/ext4/sda3/mb_groups, > since trying to convert that file over to the Church of Sysfs's style > guidelines was far more work than it was worth. I tried to find sysfs guidelines but I can not see any in Documentation speaking about the contents of the files. What are the guidelines then ? > > I'm not actually sure it's that important to be able to expose the > error function and error line number via sysfs or procfs. If a > process wants a complete record of all of the various errors, then > dmesg or maybe some netlink socket is really the best interface for > getting this information. Maybe not important, but it seems useful enough. However we might want to restrict read permissions to owner only, since it does not seem like a good idea to expose this information to the world. > > For sysfs, I suspect the primary use will be answering the questions: > "is this file system healthy or not", and "when did it first become > unhealthy". And for questoins like this, the errors_count and > first_error_time and last_error_time is probably the most useful bits > of information to expose. So you're suggesting to have three sysfs files ? errors_count first_error_time last_error_time -Lukas > > Cheers, > > - Ted > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 07, 2014 at 05:35:54PM +0200, Lukáš Czerner wrote: > > I tried to find sysfs guidelines but I can not see any in > Documentation speaking about the contents of the files. > > What are the guidelines then ? Documentation/filesystems/sysfs.txt: Attributes can be exported for kobjects in the form of regular files in the filesystem. Sysfs forwards file I/O operations to methods defined for the attributes, providing a means to read and write kernel attributes. Attributes should be ASCII text files, preferably with only one value per file. It is noted that it may not be efficient to contain only one value per file, so it is socially acceptable to express an array of values of the same type. I don't remember that last sentence; it was apparently added since the last time I've looked at it. Originally, the requirement that each sysfs file (which was supposed to be an kobject attribute) was required to be a single value. Now there's an escape hatch for "efficiency", which is nice.... > So you're suggesting to have three sysfs files ? > > errors_count > first_error_time > last_error_time Yes, that's what I was suggesting. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 7 May 2014, Theodore Ts'o wrote: > Date: Wed, 7 May 2014 12:01:52 -0400 > From: Theodore Ts'o <tytso@mit.edu> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: linux-ext4@vger.kernel.org > Subject: Re: [PATCH v2] ext4: add sysfs entry showing whether the fs contains > errors > > On Wed, May 07, 2014 at 05:35:54PM +0200, Lukáš Czerner wrote: > > > > I tried to find sysfs guidelines but I can not see any in > > Documentation speaking about the contents of the files. > > > > What are the guidelines then ? > > Documentation/filesystems/sysfs.txt: > > Attributes can be exported for kobjects in the form of regular > files in the filesystem. Sysfs forwards file I/O operations to > methods defined for the attributes, providing a means to read and > write kernel attributes. > > Attributes should be ASCII text files, preferably with only one value > per file. It is noted that it may not be efficient to contain only one > value per file, so it is socially acceptable to express an array of > values of the same type. > > I don't remember that last sentence; it was apparently added since the > last time I've looked at it. Originally, the requirement that each > sysfs file (which was supposed to be an kobject attribute) was > required to be a single value. Now there's an escape hatch for > "efficiency", which is nice.... Perfect, thanks! -Lukas > > > So you're suggesting to have three sysfs files ? > > > > errors_count > > first_error_time > > last_error_time > > Yes, that's what I was suggesting. > > - Ted >
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 6f9e6fa..9d49ec1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -2502,6 +2502,27 @@ static ssize_t lifetime_write_kbytes_show(struct ext4_attr *a, EXT4_SB(sb)->s_sectors_written_start) >> 1))); } +static ssize_t errors_show(struct ext4_attr *a, + struct ext4_sb_info *sbi, char *buf) +{ + struct ext4_super_block *es = sbi->s_es; + + if (es->s_error_count) + return snprintf(buf, PAGE_SIZE, + "%u first %u %.*s:%d last %u %.*s:%d\n", + le32_to_cpu(es->s_error_count), + le32_to_cpu(es->s_first_error_time), + (int) sizeof(es->s_first_error_func), + es->s_first_error_func, + le32_to_cpu(es->s_first_error_line), + le32_to_cpu(es->s_last_error_time), + (int) sizeof(es->s_last_error_func), + es->s_last_error_func, + le32_to_cpu(es->s_last_error_line)); + else + return snprintf(buf, PAGE_SIZE, "0\n"); +} + static ssize_t inode_readahead_blks_store(struct ext4_attr *a, struct ext4_sb_info *sbi, const char *buf, size_t count) @@ -2617,6 +2638,7 @@ static struct ext4_attr ext4_attr_##_name = { \ EXT4_RO_ATTR(delayed_allocation_blocks); EXT4_RO_ATTR(session_write_kbytes); EXT4_RO_ATTR(lifetime_write_kbytes); +EXT4_RO_ATTR(errors); EXT4_RW_ATTR(reserved_clusters); EXT4_ATTR_OFFSET(inode_readahead_blks, 0644, sbi_ui_show, inode_readahead_blks_store, s_inode_readahead_blks); @@ -2641,6 +2663,7 @@ static struct attribute *ext4_attrs[] = { ATTR_LIST(delayed_allocation_blocks), ATTR_LIST(session_write_kbytes), ATTR_LIST(lifetime_write_kbytes), + ATTR_LIST(errors), ATTR_LIST(reserved_clusters), ATTR_LIST(inode_readahead_blks), ATTR_LIST(inode_goal),
Currently there is no easy way to tell that the mounted file system contains errors other than checking for log messages, or reading the information directly from superblock. This patch adds new sysfs entry "errors" for each ext4 file system so user can simply check cat /sys/fs/ext4/sda/errors If the file system is not marked as containing errors then the file returns just 0. Otherwise it would print out the following information: <error count> first <first_error_time> <first_error_func>:<first_error_line> \ last <last_error_time> <last_error_func>:<last_error_line> For example: 2 first 1399305407 trigger_test_error:2630 last 1399463224 trigger_test_error:2601 Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- v2: Change the name of the file to errors and change the output format fs/ext4/super.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)