linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] FAT errors, user space notifications
@ 2009-06-01 14:28 Denis Karpov
  2009-06-01 14:28 ` [PATCH 1/5] FAT: add 'errors' mount option Denis Karpov
  2009-06-03  3:08 ` [PATCH 0/5] FAT errors, user space notifications OGAWA Hirofumi
  0 siblings, 2 replies; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy


Hello,

here's a series of patches that implement:

1. Options for FAT file system behavior on errors (continue, panic, 
   remount r/o)

   Current FAT behavior is to remount itself read-only on critical errors.
   Quite often this causes more harm to user space applications than if the
   error would be ignored - file system suddenly becoming r/o leads to all 
   kind of surprises from applications (yes, I know applications should be
   written properly, this is not always the case). 

   'errors' mount option (equivalent to the one in 
   ext2 fs) offers possibility for user space to specify the desired behavior.
   Default behavior is still as it was: remount read-only.
   [PATCH 1]

2. Generic mechanism for notifications of user space about file system's 
   errors/inconsistency on a particular partition using:

     - sysfs entry /sys/block/<bdev>/<part>/fs_unclean
     - uevent KOBJ_CHANGE, uevent's environment variable FS_UNCLEAN=[0:1]

   User space might want to monitor these notifications (poll2() on sysfs
   file or udevd's rule for uevent) and fix the fs damage.
   File system can be marked clean again by writing '0' to the corresponding 
   'fs_unclean' sysfs file.

   Reason for this feature: doing full scale fsck on a file system 
   at mounting time (especially residing on a slow and error prone media 
   such as flash) takes long. Full fsck results e.g. in slow boot times.
   Alternative approach is to run limited fsck (or none at all) at 
   mounting/boot time. At run-rime if an fs error is encountered, notify 
   the user space and expect it to fix the file system.
   [PATCH 2]
   
3. Make FAT and EXT2 file systems use the above mechanism to optionally 
   notify user space about errors. Implemented as 'notify' mount option.
   FAT error reporting facilities had to be re-factored in order to 
   simplify sending error notifications.
   [PATCH 3,4,5]

Adrian Hunter and Artem Bityutskiy provided input and ideas on implementing
these features.

Denis Karpov.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] FAT: add 'errors' mount option
  2009-06-01 14:28 [PATCH 0/5] FAT errors, user space notifications Denis Karpov
@ 2009-06-01 14:28 ` Denis Karpov
  2009-06-01 14:28   ` [PATCH 2/5] FS: filesystem corruption notification Denis Karpov
  2009-06-03  2:49   ` [PATCH 1/5] FAT: add 'errors' " OGAWA Hirofumi
  2009-06-03  3:08 ` [PATCH 0/5] FAT errors, user space notifications OGAWA Hirofumi
  1 sibling, 2 replies; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

On severe errors FAT remounts itself in read-only mode. Allow to
specify FAT fs desired behavior through 'errors' mount option:
panic, continue or remount read-only.

`mount -t [fat|vfat] -o errors=[panic,remount-ro,continue] \
	<bdev> <mount point>`

This is analog to ext2 fs 'errors' mount option.

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 Documentation/filesystems/vfat.txt |    5 +++++
 fs/fat/cache.c                     |    6 +++---
 fs/fat/dir.c                       |    2 +-
 fs/fat/fat.h                       |    9 +++++++--
 fs/fat/fatent.c                    |    4 ++--
 fs/fat/file.c                      |    2 +-
 fs/fat/inode.c                     |   32 ++++++++++++++++++++++++++++++--
 fs/fat/misc.c                      |   23 +++++++++++++++--------
 fs/fat/namei_msdos.c               |    2 +-
 fs/fat/namei_vfat.c                |    2 +-
 10 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/Documentation/filesystems/vfat.txt b/Documentation/filesystems/vfat.txt
index 3a5ddc9..76d6286 100644
--- a/Documentation/filesystems/vfat.txt
+++ b/Documentation/filesystems/vfat.txt
@@ -132,6 +132,11 @@ rodir	      -- FAT has the ATTR_RO (read-only) attribute. But on Windows,
 		 If you want to use ATTR_RO as read-only flag even for
 		 the directory, set this option.
 
+errors=panic|continue|remount-ro
+	      -- specify FAT behavior on critical errors: panic, continue
+		 without doing anything or remopunt the partition in
+		 read-only mode (default behavior).
+
 <bool>: 0,1,yes,no,true,false
 
 TODO
diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index b426022..923990e 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -241,7 +241,7 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 	while (*fclus < cluster) {
 		/* prevent the infinite loop of cluster chain */
 		if (*fclus > limit) {
-			fat_fs_panic(sb, "%s: detected the cluster chain loop"
+			fat_fs_error(sb, "%s: detected the cluster chain loop"
 				     " (i_pos %lld)", __func__,
 				     MSDOS_I(inode)->i_pos);
 			nr = -EIO;
@@ -252,7 +252,7 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 		if (nr < 0)
 			goto out;
 		else if (nr == FAT_ENT_FREE) {
-			fat_fs_panic(sb, "%s: invalid cluster chain"
+			fat_fs_error(sb, "%s: invalid cluster chain"
 				     " (i_pos %lld)", __func__,
 				     MSDOS_I(inode)->i_pos);
 			nr = -EIO;
@@ -285,7 +285,7 @@ static int fat_bmap_cluster(struct inode *inode, int cluster)
 	if (ret < 0)
 		return ret;
 	else if (ret == FAT_ENT_EOF) {
-		fat_fs_panic(sb, "%s: request beyond EOF (i_pos %lld)",
+		fat_fs_error(sb, "%s: request beyond EOF (i_pos %lld)",
 			     __func__, MSDOS_I(inode)->i_pos);
 		return -EIO;
 	}
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 3a7f603..7e7924c 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1334,7 +1334,7 @@ found:
 			goto error_remove;
 		}
 		if (dir->i_size & (sbi->cluster_size - 1)) {
-			fat_fs_panic(sb, "Odd directory size");
+			fat_fs_error(sb, "Odd directory size");
 			dir->i_size = (dir->i_size + sbi->cluster_size - 1)
 				& ~((loff_t)sbi->cluster_size - 1);
 		}
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index ea440d6..80d83c1 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -17,6 +17,10 @@
 #define VFAT_SFN_CREATE_WIN95	0x0100 /* emulate win95 rule for create */
 #define VFAT_SFN_CREATE_WINNT	0x0200 /* emulate winnt rule for create */
 
+#define FAT_ERRORS_CONT		0x1
+#define FAT_ERRORS_PANIC	0x2
+#define FAT_ERRORS_RO		0x4
+
 struct fat_mount_options {
 	uid_t fs_uid;
 	gid_t fs_gid;
@@ -39,7 +43,8 @@ struct fat_mount_options {
 		 nocase:1,	  /* Does this need case conversion? 0=need case conversion*/
 		 usefree:1,	  /* Use free_clusters for FAT32 */
 		 tz_utc:1,	  /* Filesystem timestamps are in UTC */
-		 rodir:1;	  /* allow ATTR_RO for directory */
+		 rodir:1,	  /* allow ATTR_RO for directory */
+		 errors:3;	  /* On error: continue, panic, go ro */
 };
 
 #define FAT_HASH_BITS	8
@@ -310,7 +315,7 @@ extern int fat_fill_super(struct super_block *sb, void *data, int silent,
 extern int fat_flush_inodes(struct super_block *sb, struct inode *i1,
 		            struct inode *i2);
 /* fat/misc.c */
-extern void fat_fs_panic(struct super_block *s, const char *fmt, ...)
+extern void fat_fs_error(struct super_block *s, const char *fmt, ...)
 	__attribute__ ((format (printf, 2, 3))) __cold;
 extern void fat_clusters_flush(struct super_block *sb);
 extern int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster);
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index da6eea4..60c31f7 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -345,7 +345,7 @@ int fat_ent_read(struct inode *inode, struct fat_entry *fatent, int entry)
 
 	if (entry < FAT_START_ENT || sbi->max_cluster <= entry) {
 		fatent_brelse(fatent);
-		fat_fs_panic(sb, "invalid access to FAT (entry 0x%08x)", entry);
+		fat_fs_error(sb, "invalid access to FAT (entry 0x%08x)", entry);
 		return -EIO;
 	}
 
@@ -557,7 +557,7 @@ int fat_free_clusters(struct inode *inode, int cluster)
 			err = cluster;
 			goto error;
 		} else if (cluster == FAT_ENT_FREE) {
-			fat_fs_panic(sb, "%s: deleting FAT entry beyond EOF",
+			fat_fs_error(sb, "%s: deleting FAT entry beyond EOF",
 				     __func__);
 			err = -EIO;
 			goto error;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 0a7f4a9..6214287 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -213,7 +213,7 @@ static int fat_free(struct inode *inode, int skip)
 			fatent_brelse(&fatent);
 			return 0;
 		} else if (ret == FAT_ENT_FREE) {
-			fat_fs_panic(sb,
+			fat_fs_error(sb,
 				     "%s: invalid cluster chain (i_pos %lld)",
 				     __func__, MSDOS_I(inode)->i_pos);
 			ret = -EIO;
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 296785a..56340b3 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -76,7 +76,7 @@ static inline int __fat_get_block(struct inode *inode, sector_t iblock,
 		return 0;
 
 	if (iblock != MSDOS_I(inode)->mmu_private >> sb->s_blocksize_bits) {
-		fat_fs_panic(sb, "corrupted file size (i_pos %lld, %lld)",
+		fat_fs_error(sb, "corrupted file size (i_pos %lld, %lld)",
 			MSDOS_I(inode)->i_pos, MSDOS_I(inode)->mmu_private);
 		return -EIO;
 	}
@@ -834,6 +834,12 @@ static int fat_show_options(struct seq_file *m, struct vfsmount *mnt)
 		seq_puts(m, ",flush");
 	if (opts->tz_utc)
 		seq_puts(m, ",tz=UTC");
+	if (opts->errors & FAT_ERRORS_CONT)
+		seq_puts(m, ",errors=continue");
+	if (opts->errors & FAT_ERRORS_PANIC)
+		seq_puts(m, ",errors=panic");
+	if (opts->errors & FAT_ERRORS_RO)
+		seq_puts(m, ",errors=ro");
 
 	return 0;
 }
@@ -846,7 +852,8 @@ enum {
 	Opt_charset, Opt_shortname_lower, Opt_shortname_win95,
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
-	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err,
+	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
+	Opt_err_panic, Opt_err_ro, Opt_err,
 };
 
 static const match_table_t fat_tokens = {
@@ -882,6 +889,9 @@ static const match_table_t fat_tokens = {
 	{Opt_obsolate, "posix"},
 	{Opt_flush, "flush"},
 	{Opt_tz_utc, "tz=UTC"},
+	{Opt_err_cont, "errors=continue"},
+	{Opt_err_panic, "errors=panic"},
+	{Opt_err_ro, "errors=remount-ro"},
 	{Opt_err, NULL},
 };
 static const match_table_t msdos_tokens = {
@@ -889,6 +899,9 @@ static const match_table_t msdos_tokens = {
 	{Opt_nodots, "dotsOK=no"},
 	{Opt_dots, "dots"},
 	{Opt_dots, "dotsOK=yes"},
+	{Opt_err_cont, "errors=continue"},
+	{Opt_err_panic, "errors=panic"},
+	{Opt_err_ro, "errors=remount-ro"},
 	{Opt_err, NULL}
 };
 static const match_table_t vfat_tokens = {
@@ -919,6 +932,9 @@ static const match_table_t vfat_tokens = {
 	{Opt_nonumtail_yes, "nonumtail=true"},
 	{Opt_nonumtail_yes, "nonumtail"},
 	{Opt_rodir, "rodir"},
+	{Opt_err_cont, "errors=continue"},
+	{Opt_err_panic, "errors=panic"},
+	{Opt_err_ro, "errors=remount-ro"},
 	{Opt_err, NULL}
 };
 
@@ -1043,6 +1059,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		case Opt_tz_utc:
 			opts->tz_utc = 1;
 			break;
+		case Opt_err_cont:
+			opts->errors = FAT_ERRORS_CONT;
+			break;
+		case Opt_err_panic:
+			opts->errors = FAT_ERRORS_PANIC;
+			break;
+		case Opt_err_ro:
+			opts->errors = FAT_ERRORS_RO;
+			break;
 
 		/* msdos specific */
 		case Opt_dots:
@@ -1128,6 +1153,9 @@ out:
 		opts->allow_utime = ~opts->fs_dmask & (S_IWGRP | S_IWOTH);
 	if (opts->unicode_xlate)
 		opts->utf8 = 0;
+	/* Default behavior on fs errors - go r/o */
+	if (!opts->errors)
+		opts->errors = FAT_ERRORS_RO;
 
 	return 0;
 }
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index ac39ebc..7e78618 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -12,14 +12,19 @@
 #include "fat.h"
 
 /*
- * fat_fs_panic reports a severe file system problem and sets the file system
- * read-only. The file system can be made writable again by remounting it.
+ * fat_fs_error reports a file system problem that might indicate fa data
+ * corruption/inconsistency. Depending on 'errors' mount option the
+ * panic() is called, or error message is printed FAT and nothing is done,
+ * or filesystem is remounted read-only (default behavior).
+ * In case the file system is remounted read-only, it can be made writable
+ * again by remounting it.
  */
-void fat_fs_panic(struct super_block *s, const char *fmt, ...)
+void fat_fs_error(struct super_block *s, const char *fmt, ...)
 {
 	va_list args;
+	struct msdos_sb_info *sbi = MSDOS_SB(s);
 
-	printk(KERN_ERR "FAT: Filesystem panic (dev %s)\n", s->s_id);
+	printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
 
 	printk(KERN_ERR "    ");
 	va_start(args, fmt);
@@ -27,13 +32,15 @@ void fat_fs_panic(struct super_block *s, const char *fmt, ...)
 	va_end(args);
 	printk("\n");
 
-	if (!(s->s_flags & MS_RDONLY)) {
+	if (sbi->options.errors & FAT_ERRORS_PANIC)
+		panic("    FAT fs panic from previous error\n");
+	if ((sbi->options.errors & FAT_ERRORS_RO) &&
+				!(s->s_flags & MS_RDONLY)) {
 		s->s_flags |= MS_RDONLY;
 		printk(KERN_ERR "    File system has been set read-only\n");
 	}
 }
-
-EXPORT_SYMBOL_GPL(fat_fs_panic);
+EXPORT_SYMBOL_GPL(fat_fs_error);
 
 /* Flushes the number of free clusters on FAT32 */
 /* XXX: Need to write one per FSINFO block.  Currently only writes 1 */
@@ -124,7 +131,7 @@ int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster)
 			mark_inode_dirty(inode);
 	}
 	if (new_fclus != (inode->i_blocks >> (sbi->cluster_bits - 9))) {
-		fat_fs_panic(sb, "clusters badly computed (%d != %llu)",
+		fat_fs_error(sb, "clusters badly computed (%d != %llu)",
 			     new_fclus,
 			     (llu)(inode->i_blocks >> (sbi->cluster_bits - 9)));
 		fat_cache_inval_inode(inode);
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index da3f361..72f5c64 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -608,7 +608,7 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_panic(new_dir->i_sb,
+		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, sinfo.i_pos);
 	}
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index a0e00e3..cb6ddb8 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1030,7 +1030,7 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_panic(new_dir->i_sb,
+		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, sinfo.i_pos);
 	}
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] FS: filesystem corruption notification
  2009-06-01 14:28 ` [PATCH 1/5] FAT: add 'errors' mount option Denis Karpov
@ 2009-06-01 14:28   ` Denis Karpov
  2009-06-01 14:28     ` [PATCH 3/5] FAT: generalize errors and warning printing Denis Karpov
  2009-06-03  2:49   ` [PATCH 1/5] FAT: add 'errors' " OGAWA Hirofumi
  1 sibling, 1 reply; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Add a generic mechnism to notify the userspace about possible filesystem
corruption through sysfs entry (/sys/block/<bdev>/<part>/fs_unclean)
and uevent (KOBJ_CHANGE, uevent's environment variable FS_UNCLEAN=[0:1]).

To mark fs clean (e.g. after fs was fixed by userspace):

echo 0 > /sys/block/<bdev>/<part>/fs_unclean
(you will still receive uevent KOBJ_CHANGE with env var FS_UNCLEAN=0)

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 fs/partitions/check.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/genhd.h |    2 ++
 2 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 99e33ef..e1a6221 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -196,6 +196,24 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 	return ERR_PTR(res);
 }
 
+void notify_part_fs_unclean(struct device *dev, uint8_t unclean)
+{
+	char event_string[11];
+	char *envp[] = { event_string, NULL };
+
+	if ((unclean != 0 && unclean != 1) ||
+		unclean == dev_to_part(dev)->fs_unclean)
+		return;
+
+	dev_to_part(dev)->fs_unclean = unclean;
+
+	sysfs_notify(&dev->kobj, NULL, "fs_unclean");
+
+	sprintf(event_string, "FS_DIRTY=%u", unclean);
+	kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp);
+}
+EXPORT_SYMBOL(notify_part_fs_unclean);
+
 static ssize_t part_partition_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
@@ -246,6 +264,26 @@ ssize_t part_stat_show(struct device *dev,
 		jiffies_to_msecs(part_stat_read(p, time_in_queue)));
 }
 
+ssize_t part_fs_unclean_show(struct device *dev,
+		       struct device_attribute *attr, char *buf)
+{
+	struct hd_struct *p = dev_to_part(dev);
+
+	return sprintf(buf, "%d\n", p->fs_unclean);
+}
+
+ssize_t part_fs_unclean_store(struct device *dev,
+			struct device_attribute *attr,
+			const char *buf, size_t count)
+{
+	int i;
+
+	if (count > 0 && sscanf(buf, "%d", &i) > 0)
+		notify_part_fs_unclean(dev, (i == 0) ? 0 : 1);
+
+	return count;
+}
+
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 ssize_t part_fail_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
@@ -273,6 +311,9 @@ static DEVICE_ATTR(partition, S_IRUGO, part_partition_show, NULL);
 static DEVICE_ATTR(start, S_IRUGO, part_start_show, NULL);
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
+static struct device_attribute dev_attr_fs_unclean =
+	__ATTR(fs_unclean, S_IRUGO|S_IWUSR, part_fs_unclean_show,
+		part_fs_unclean_store);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 static struct device_attribute dev_attr_fail =
 	__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -283,6 +324,7 @@ static struct attribute *part_attrs[] = {
 	&dev_attr_start.attr,
 	&dev_attr_size.attr,
 	&dev_attr_stat.attr,
+	&dev_attr_fs_unclean.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&dev_attr_fail.attr,
 #endif
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index a1a28ca..2e6d42e 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -98,6 +98,7 @@ struct hd_struct {
 #endif
 	unsigned long stamp;
 	int in_flight;
+	int fs_unclean;
 #ifdef	CONFIG_SMP
 	struct disk_stats *dkstats;
 #else
@@ -528,6 +529,7 @@ extern struct hd_struct * __must_check add_partition(struct gendisk *disk,
 						     sector_t len, int flags);
 extern void delete_partition(struct gendisk *, int);
 extern void printk_all_partitions(void);
+extern void notify_part_fs_unclean(struct device *dev, uint8_t unclean);
 
 extern struct gendisk *alloc_disk_node(int minors, int node_id);
 extern struct gendisk *alloc_disk(int minors);
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] FAT: generalize errors and warning printing
  2009-06-01 14:28   ` [PATCH 2/5] FS: filesystem corruption notification Denis Karpov
@ 2009-06-01 14:28     ` Denis Karpov
  2009-06-01 14:28       ` [PATCH 4/5] FAT: add 'notify' mount option Denis Karpov
  0 siblings, 1 reply; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Generalize FAT errors and warnings reporting through
fat_fs_error() and fat_fs_warning().

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 fs/fat/cache.c       |   14 ++++----
 fs/fat/dir.c         |   11 ++++---
 fs/fat/fat.h         |    8 ++++-
 fs/fat/fatent.c      |   15 +++++----
 fs/fat/file.c        |    6 ++--
 fs/fat/inode.c       |   81 ++++++++++++++++++++++++++++---------------------
 fs/fat/misc.c        |   41 +++++++++++++++++++-----
 fs/fat/namei_msdos.c |    6 ++--
 fs/fat/namei_vfat.c  |    6 ++--
 9 files changed, 115 insertions(+), 73 deletions(-)

diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index 923990e..b1cf11d 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -241,9 +241,9 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 	while (*fclus < cluster) {
 		/* prevent the infinite loop of cluster chain */
 		if (*fclus > limit) {
-			fat_fs_error(sb, "%s: detected the cluster chain loop"
-				     " (i_pos %lld)", __func__,
-				     MSDOS_I(inode)->i_pos);
+			fat_fs_error(sb, __func__, "detected the cluster "
+				"chain loop (i_pos %lld)",
+				MSDOS_I(inode)->i_pos);
 			nr = -EIO;
 			goto out;
 		}
@@ -252,8 +252,8 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 		if (nr < 0)
 			goto out;
 		else if (nr == FAT_ENT_FREE) {
-			fat_fs_error(sb, "%s: invalid cluster chain"
-				     " (i_pos %lld)", __func__,
+			fat_fs_error(sb, __func__, "invalid cluster chain"
+				     " (i_pos %lld)",
 				     MSDOS_I(inode)->i_pos);
 			nr = -EIO;
 			goto out;
@@ -285,8 +285,8 @@ static int fat_bmap_cluster(struct inode *inode, int cluster)
 	if (ret < 0)
 		return ret;
 	else if (ret == FAT_ENT_EOF) {
-		fat_fs_error(sb, "%s: request beyond EOF (i_pos %lld)",
-			     __func__, MSDOS_I(inode)->i_pos);
+		fat_fs_error(sb, __func__, "request beyond EOF (i_pos %lld)",
+			MSDOS_I(inode)->i_pos);
 		return -EIO;
 	}
 	return dclus;
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 7e7924c..390a984 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -85,8 +85,9 @@ next:
 
 	*bh = sb_bread(sb, phys);
 	if (*bh == NULL) {
-		printk(KERN_ERR "FAT: Directory bread(block %llu) failed\n",
-		       (llu)phys);
+		fat_fs_warning(sb, __func__, "Directory bread(block %llu) "
+			"failed",
+			(llu)phys);
 		/* skip this block */
 		*pos = (iblock + 1) << sb->s_blocksize_bits;
 		goto next;
@@ -1269,8 +1270,8 @@ int fat_add_entries(struct inode *dir, void *slots, int nr_slots,
 		if (sbi->fat_bits != 32)
 			goto error;
 	} else if (MSDOS_I(dir)->i_start == 0) {
-		printk(KERN_ERR "FAT: Corrupted directory (i_pos %lld)\n",
-		       MSDOS_I(dir)->i_pos);
+		fat_fs_warning(sb, __func__, "Corrupted directory (i_pos %lld)",
+			MSDOS_I(dir)->i_pos);
 		err = -EIO;
 		goto error;
 	}
@@ -1334,7 +1335,7 @@ found:
 			goto error_remove;
 		}
 		if (dir->i_size & (sbi->cluster_size - 1)) {
-			fat_fs_error(sb, "Odd directory size");
+			fat_fs_error(sb, __func__, "Odd directory size");
 			dir->i_size = (dir->i_size + sbi->cluster_size - 1)
 				& ~((loff_t)sbi->cluster_size - 1);
 		}
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 80d83c1..d6fd6ab 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -315,8 +315,12 @@ extern int fat_fill_super(struct super_block *sb, void *data, int silent,
 extern int fat_flush_inodes(struct super_block *sb, struct inode *i1,
 		            struct inode *i2);
 /* fat/misc.c */
-extern void fat_fs_error(struct super_block *s, const char *fmt, ...)
-	__attribute__ ((format (printf, 2, 3))) __cold;
+extern void fat_fs_error(struct super_block *s, const char *function,
+			const char *fmt, ...)
+	__attribute__ ((format (printf, 3, 4))) __cold;
+extern void fat_fs_warning(struct super_block *s, const char *function,
+			const char *fmt, ...)
+	__attribute__ ((format (printf, 3, 4))) __cold;
 extern void fat_clusters_flush(struct super_block *sb);
 extern int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster);
 extern void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index 60c31f7..13f2de9 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -93,7 +93,8 @@ static int fat12_ent_bread(struct super_block *sb, struct fat_entry *fatent,
 err_brelse:
 	brelse(bhs[0]);
 err:
-	printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n", (llu)blocknr);
+	fat_fs_warning(sb, __func__, "FAT read failed (blocknr %llu)",
+		(llu)blocknr);
 	return -EIO;
 }
 
@@ -105,8 +106,9 @@ static int fat_ent_bread(struct super_block *sb, struct fat_entry *fatent,
 	WARN_ON(blocknr < MSDOS_SB(sb)->fat_start);
 	fatent->bhs[0] = sb_bread(sb, blocknr);
 	if (!fatent->bhs[0]) {
-		printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n",
-		       (llu)blocknr);
+		fat_fs_warning(sb, __func__, "FAT: FAT read failed "
+			"(blocknr %llu)",
+			(llu)blocknr);
 		return -EIO;
 	}
 	fatent->nr_bhs = 1;
@@ -345,7 +347,8 @@ int fat_ent_read(struct inode *inode, struct fat_entry *fatent, int entry)
 
 	if (entry < FAT_START_ENT || sbi->max_cluster <= entry) {
 		fatent_brelse(fatent);
-		fat_fs_error(sb, "invalid access to FAT (entry 0x%08x)", entry);
+		fat_fs_error(sb, __func__, "invalid access to FAT "
+			"(entry 0x%08x)", entry);
 		return -EIO;
 	}
 
@@ -557,8 +560,8 @@ int fat_free_clusters(struct inode *inode, int cluster)
 			err = cluster;
 			goto error;
 		} else if (cluster == FAT_ENT_FREE) {
-			fat_fs_error(sb, "%s: deleting FAT entry beyond EOF",
-				     __func__);
+			fat_fs_error(sb, __func__, "deleting FAT entry beyond "
+				"EOF");
 			err = -EIO;
 			goto error;
 		}
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 6214287..df65446 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -213,9 +213,9 @@ static int fat_free(struct inode *inode, int skip)
 			fatent_brelse(&fatent);
 			return 0;
 		} else if (ret == FAT_ENT_FREE) {
-			fat_fs_error(sb,
-				     "%s: invalid cluster chain (i_pos %lld)",
-				     __func__, MSDOS_I(inode)->i_pos);
+			fat_fs_error(sb, __func__,
+				     "invalid cluster chain (i_pos %lld)",
+				     MSDOS_I(inode)->i_pos);
 			ret = -EIO;
 		} else if (ret > 0) {
 			err = fat_ent_write(inode, &fatent, FAT_ENT_EOF, wait);
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 56340b3..40fcf97 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -76,7 +76,8 @@ static inline int __fat_get_block(struct inode *inode, sector_t iblock,
 		return 0;
 
 	if (iblock != MSDOS_I(inode)->mmu_private >> sb->s_blocksize_bits) {
-		fat_fs_error(sb, "corrupted file size (i_pos %lld, %lld)",
+		fat_fs_error(sb, __func__, "corrupted file size "
+			"(i_pos %lld, %lld)",
 			MSDOS_I(inode)->i_pos, MSDOS_I(inode)->mmu_private);
 		return -EIO;
 	}
@@ -579,8 +580,8 @@ retry:
 
 	bh = sb_bread(sb, i_pos >> sbi->dir_per_block_bits);
 	if (!bh) {
-		printk(KERN_ERR "FAT: unable to read inode block "
-		       "for updating (i_pos %lld)\n", i_pos);
+		fat_fs_warning(sb, __func__, "unable to read inode block "
+		       "for updating (i_pos %lld)", i_pos);
 		return -EIO;
 	}
 	spin_lock(&sbi->inode_hash_lock);
@@ -1132,9 +1133,8 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		/* unknown option */
 		default:
 			if (!silent) {
-				printk(KERN_ERR
-				       "FAT: Unrecognized mount option \"%s\" "
-				       "or missing value\n", p);
+				printk(KERN_ERR "FAT: Unrecognized mount "
+					"option \"%s\" or missing value\n", p);
 			}
 			return -EINVAL;
 		}
@@ -1143,9 +1143,9 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 out:
 	/* UTF-8 doesn't provide FAT semantics */
 	if (!strcmp(opts->iocharset, "utf8")) {
-		printk(KERN_ERR "FAT: utf8 is not a recommended IO charset"
-		       " for FAT filesystems, filesystem will be "
-		       "case sensitive!\n");
+		printk(KERN_WARNING "FAT: utf8 is not a recommended IO "
+			"charset for FAT filesystems, filesystem will be "
+			"case sensitive!");
 	}
 
 	/* If user doesn't specify allow_utime, it's initialized from dmask. */
@@ -1238,20 +1238,22 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	sb_min_blocksize(sb, 512);
 	bh = sb_bread(sb, 0);
 	if (bh == NULL) {
-		printk(KERN_ERR "FAT: unable to read boot sector\n");
+		fat_fs_warning(sb, __func__, "unable to read boot sector");
 		goto out_fail;
 	}
 
 	b = (struct fat_boot_sector *) bh->b_data;
 	if (!b->reserved) {
 		if (!silent)
-			printk(KERN_ERR "FAT: bogus number of reserved sectors\n");
+			fat_fs_warning(sb, __func__, "bogus number of reserved "
+					"sectors");
 		brelse(bh);
 		goto out_invalid;
 	}
 	if (!b->fats) {
 		if (!silent)
-			printk(KERN_ERR "FAT: bogus number of FAT structure\n");
+			fat_fs_warning(sb, __func__, "bogus number of FAT "
+					"structure");
 		brelse(bh);
 		goto out_invalid;
 	}
@@ -1264,8 +1266,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	media = b->media;
 	if (!fat_valid_media(media)) {
 		if (!silent)
-			printk(KERN_ERR "FAT: invalid media value (0x%02x)\n",
-			       media);
+			fat_fs_warning(sb, __func__, "invalid media value "
+				"(0x%02x)",
+				media);
 		brelse(bh);
 		goto out_invalid;
 	}
@@ -1274,23 +1277,26 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	    || (logical_sector_size < 512)
 	    || (logical_sector_size > 4096)) {
 		if (!silent)
-			printk(KERN_ERR "FAT: bogus logical sector size %u\n",
-			       logical_sector_size);
+			fat_fs_warning(sb, __func__, "bogus logical sector "
+				"size %u",
+				logical_sector_size);
 		brelse(bh);
 		goto out_invalid;
 	}
 	sbi->sec_per_clus = b->sec_per_clus;
 	if (!is_power_of_2(sbi->sec_per_clus)) {
 		if (!silent)
-			printk(KERN_ERR "FAT: bogus sectors per cluster %u\n",
-			       sbi->sec_per_clus);
+			fat_fs_warning(sb, __func__, "bogus sectors per "
+				"cluster %u",
+				sbi->sec_per_clus);
 		brelse(bh);
 		goto out_invalid;
 	}
 
 	if (logical_sector_size < sb->s_blocksize) {
-		printk(KERN_ERR "FAT: logical sector size too small for device"
-		       " (logical sector size = %u)\n", logical_sector_size);
+		fat_fs_warning(sb, __func__, "logical sector size too small "
+			"for device"
+		       " (logical sector size = %u)", logical_sector_size);
 		brelse(bh);
 		goto out_fail;
 	}
@@ -1298,15 +1304,16 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 		brelse(bh);
 
 		if (!sb_set_blocksize(sb, logical_sector_size)) {
-			printk(KERN_ERR "FAT: unable to set blocksize %u\n",
-			       logical_sector_size);
+			fat_fs_warning(sb, __func__, "unable to set "
+				"blocksize %u",
+				logical_sector_size);
 			goto out_fail;
 		}
 		bh = sb_bread(sb, 0);
 		if (bh == NULL) {
-			printk(KERN_ERR "FAT: unable to read boot sector"
-			       " (logical sector size = %lu)\n",
-			       sb->s_blocksize);
+			fat_fs_warning(sb, __func__, "unable to read "
+				"boot sector (logical sector size = %lu)",
+				sb->s_blocksize);
 			goto out_fail;
 		}
 		b = (struct fat_boot_sector *) bh->b_data;
@@ -1341,8 +1348,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 
 		fsinfo_bh = sb_bread(sb, sbi->fsinfo_sector);
 		if (fsinfo_bh == NULL) {
-			printk(KERN_ERR "FAT: bread failed, FSINFO block"
-			       " (sector = %lu)\n", sbi->fsinfo_sector);
+			fat_fs_warning(sb, __func__, "bread failed, FSINFO "
+				"block (sector = %lu)",
+				sbi->fsinfo_sector);
 			brelse(bh);
 			goto out_fail;
 		}
@@ -1371,8 +1379,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	sbi->dir_entries = get_unaligned_le16(&b->dir_entries);
 	if (sbi->dir_entries & (sbi->dir_per_block - 1)) {
 		if (!silent)
-			printk(KERN_ERR "FAT: bogus directroy-entries per block"
-			       " (%u)\n", sbi->dir_entries);
+			fat_fs_warning(sb, __func__, "bogus directroy-entries"
+				" per block (%u)",
+				sbi->dir_entries);
 		brelse(bh);
 		goto out_invalid;
 	}
@@ -1394,8 +1403,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	total_clusters = min(total_clusters, fat_clusters - FAT_START_ENT);
 	if (total_clusters > MAX_FAT(sb)) {
 		if (!silent)
-			printk(KERN_ERR "FAT: count of clusters too big (%u)\n",
-			       total_clusters);
+			fat_fs_warning(sb, __func__, "count of clusters too "
+				"big (%u)",
+				total_clusters);
 		brelse(bh);
 		goto out_invalid;
 	}
@@ -1427,7 +1437,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	sprintf(buf, "cp%d", sbi->options.codepage);
 	sbi->nls_disk = load_nls(buf);
 	if (!sbi->nls_disk) {
-		printk(KERN_ERR "FAT: codepage %s not found\n", buf);
+		fat_fs_warning(sb, __func__, "codepage %s not found", buf);
 		goto out_fail;
 	}
 
@@ -1435,8 +1445,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	if (sbi->options.isvfat) {
 		sbi->nls_io = load_nls(sbi->options.iocharset);
 		if (!sbi->nls_io) {
-			printk(KERN_ERR "FAT: IO charset %s not found\n",
-			       sbi->options.iocharset);
+			fat_fs_warning(sb, __func__, "IO charset %s not "
+				"found",
+				sbi->options.iocharset);
 			goto out_fail;
 		}
 	}
@@ -1454,7 +1465,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
 	insert_inode_hash(root_inode);
 	sb->s_root = d_alloc_root(root_inode);
 	if (!sb->s_root) {
-		printk(KERN_ERR "FAT: get root inode failed\n");
+		fat_fs_warning(sb, __func__, "get root inode failed");
 		goto out_fail;
 	}
 
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 7e78618..1120094 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -19,12 +19,14 @@
  * In case the file system is remounted read-only, it can be made writable
  * again by remounting it.
  */
-void fat_fs_error(struct super_block *s, const char *fmt, ...)
+void fat_fs_error(struct super_block *s, const char * function,
+			const char *fmt, ...)
 {
 	va_list args;
 	struct msdos_sb_info *sbi = MSDOS_SB(s);
 
-	printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
+	printk(KERN_ERR "FAT: Filesystem error (dev %s): %s:\n", s->s_id,
+		function);
 
 	printk(KERN_ERR "    ");
 	va_start(args, fmt);
@@ -42,6 +44,26 @@ void fat_fs_error(struct super_block *s, const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(fat_fs_error);
 
+/*
+ * fat_fs_warning reports a file system non-critical problem that still
+ * might indicate fs data corruption/inconsistency.
+ */
+void fat_fs_warning(struct super_block *s, const char * function,
+			const char *fmt, ...)
+{
+	va_list args;
+
+	printk(KERN_ERR "FAT: Filesystem warning (dev %s): %s:\n", s->s_id,
+		function);
+
+	printk(KERN_ERR "    ");
+	va_start(args, fmt);
+	vprintk(fmt, args);
+	printk("\n");
+	va_end(args);
+}
+EXPORT_SYMBOL_GPL(fat_fs_warning);
+
 /* Flushes the number of free clusters on FAT32 */
 /* XXX: Need to write one per FSINFO block.  Currently only writes 1 */
 void fat_clusters_flush(struct super_block *sb)
@@ -55,15 +77,15 @@ void fat_clusters_flush(struct super_block *sb)
 
 	bh = sb_bread(sb, sbi->fsinfo_sector);
 	if (bh == NULL) {
-		printk(KERN_ERR "FAT: bread failed in fat_clusters_flush\n");
+		fat_fs_warning(sb, __func__, "bread failed");
 		return;
 	}
 
 	fsinfo = (struct fat_boot_fsinfo *)bh->b_data;
 	/* Sanity check */
 	if (!IS_FSINFO(fsinfo)) {
-		printk(KERN_ERR "FAT: Invalid FSINFO signature: "
-		       "0x%08x, 0x%08x (sector = %lu)\n",
+		fat_fs_warning(sb, __func__, "Invalid FSINFO signature: "
+		       "0x%08x, 0x%08x (sector = %lu)",
 		       le32_to_cpu(fsinfo->signature1),
 		       le32_to_cpu(fsinfo->signature2),
 		       sbi->fsinfo_sector);
@@ -131,10 +153,11 @@ int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster)
 			mark_inode_dirty(inode);
 	}
 	if (new_fclus != (inode->i_blocks >> (sbi->cluster_bits - 9))) {
-		fat_fs_error(sb, "clusters badly computed (%d != %llu)",
-			     new_fclus,
-			     (llu)(inode->i_blocks >> (sbi->cluster_bits - 9)));
-		fat_cache_inval_inode(inode);
+		fat_fs_error(sb, __func__, "clusters badly computed "
+			"(%d != %llu)",
+			new_fclus,
+			(llu)(inode->i_blocks >> (sbi->cluster_bits - 9)));
+			fat_cache_inval_inode(inode);
 	}
 	inode->i_blocks += nr_cluster << (sbi->cluster_bits - 9);
 
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index 72f5c64..964f378 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -608,9 +608,9 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_error(new_dir->i_sb,
-			     "%s: Filesystem corrupted (i_pos %lld)",
-			     __func__, sinfo.i_pos);
+		fat_fs_error(new_dir->i_sb, __func__,
+			     "Filesystem corrupted (i_pos %lld)",
+			     sinfo.i_pos);
 	}
 	goto out;
 }
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index cb6ddb8..91601d6 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1030,9 +1030,9 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_error(new_dir->i_sb,
-			     "%s: Filesystem corrupted (i_pos %lld)",
-			     __func__, sinfo.i_pos);
+		fat_fs_error(new_dir->i_sb, __func__,
+			     "Filesystem corrupted (i_pos %lld)",
+			     sinfo.i_pos);
 	}
 	goto out;
 }
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] FAT: add 'notify' mount option
  2009-06-01 14:28     ` [PATCH 3/5] FAT: generalize errors and warning printing Denis Karpov
@ 2009-06-01 14:28       ` Denis Karpov
  2009-06-01 14:28         ` [PATCH 5/5] EXT2: " Denis Karpov
  0 siblings, 1 reply; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Implement FAT fs mount option 'notify'. The effect of this option
is that a notification is sent to userspace on errors that indicate
filesystem damage/inconsistency. Generic filesystem corruption
notification mechnism is used.

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 fs/fat/fat.h   |    3 ++-
 fs/fat/inode.c |   12 ++++++++++--
 fs/fat/misc.c  |    7 +++++++
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index d6fd6ab..1fb22ec 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -44,7 +44,8 @@ struct fat_mount_options {
 		 usefree:1,	  /* Use free_clusters for FAT32 */
 		 tz_utc:1,	  /* Filesystem timestamps are in UTC */
 		 rodir:1,	  /* allow ATTR_RO for directory */
-		 errors:3;	  /* On error: continue, panic, go ro */
+		 errors:3,	  /* On error: continue, panic, go ro */
+		 err_notify:1;	  /* Notify userspace on fs errors */
 };
 
 #define FAT_HASH_BITS	8
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 40fcf97..08b68bd 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -840,7 +840,9 @@ static int fat_show_options(struct seq_file *m, struct vfsmount *mnt)
 	if (opts->errors & FAT_ERRORS_PANIC)
 		seq_puts(m, ",errors=panic");
 	if (opts->errors & FAT_ERRORS_RO)
-		seq_puts(m, ",errors=ro");
+		seq_puts(m, ",errors=remount-ro");
+	if (opts->err_notify)
+		seq_puts(m, ",notify");
 
 	return 0;
 }
@@ -854,7 +856,7 @@ enum {
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
 	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
-	Opt_err_panic, Opt_err_ro, Opt_err,
+	Opt_err_panic, Opt_err_ro, Opt_err_notify, Opt_err,
 };
 
 static const match_table_t fat_tokens = {
@@ -893,6 +895,7 @@ static const match_table_t fat_tokens = {
 	{Opt_err_cont, "errors=continue"},
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
+	{Opt_err_notify, "notify"},
 	{Opt_err, NULL},
 };
 static const match_table_t msdos_tokens = {
@@ -903,6 +906,7 @@ static const match_table_t msdos_tokens = {
 	{Opt_err_cont, "errors=continue"},
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
+	{Opt_err_notify, "notify"},
 	{Opt_err, NULL}
 };
 static const match_table_t vfat_tokens = {
@@ -936,6 +940,7 @@ static const match_table_t vfat_tokens = {
 	{Opt_err_cont, "errors=continue"},
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
+	{Opt_err_notify, "notify"},
 	{Opt_err, NULL}
 };
 
@@ -1069,6 +1074,9 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		case Opt_err_ro:
 			opts->errors = FAT_ERRORS_RO;
 			break;
+		case Opt_err_notify:
+			opts->err_notify = 1;
+			break;
 
 		/* msdos specific */
 		case Opt_dots:
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 1120094..3143a25 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/buffer_head.h>
+#include <linux/genhd.h>
 #include "fat.h"
 
 /*
@@ -41,6 +42,8 @@ void fat_fs_error(struct super_block *s, const char * function,
 		s->s_flags |= MS_RDONLY;
 		printk(KERN_ERR "    File system has been set read-only\n");
 	}
+	if (sbi->options.err_notify)
+		notify_part_fs_unclean(part_to_dev(s->s_bdev->bd_part), 1);
 }
 EXPORT_SYMBOL_GPL(fat_fs_error);
 
@@ -52,6 +55,7 @@ void fat_fs_warning(struct super_block *s, const char * function,
 			const char *fmt, ...)
 {
 	va_list args;
+	struct msdos_sb_info *sbi = MSDOS_SB(s);
 
 	printk(KERN_ERR "FAT: Filesystem warning (dev %s): %s:\n", s->s_id,
 		function);
@@ -61,6 +65,9 @@ void fat_fs_warning(struct super_block *s, const char * function,
 	vprintk(fmt, args);
 	printk("\n");
 	va_end(args);
+
+	if (sbi->options.err_notify)
+		notify_part_fs_unclean(part_to_dev(s->s_bdev->bd_part), 1);
 }
 EXPORT_SYMBOL_GPL(fat_fs_warning);
 
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] EXT2: add 'notify' mount option
  2009-06-01 14:28       ` [PATCH 4/5] FAT: add 'notify' mount option Denis Karpov
@ 2009-06-01 14:28         ` Denis Karpov
  0 siblings, 0 replies; 12+ messages in thread
From: Denis Karpov @ 2009-06-01 14:28 UTC (permalink / raw)
  To: hirofumi; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Implement EXT2 fs mount option 'notify'. The effect of this option
is that a notification is sent to userspace on errors that indicate
filesystem damage/inconsistency. Generic filesystem corruption
notification mechnism is used.

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 fs/ext2/super.c         |   15 ++++++++++++++-
 include/linux/ext2_fs.h |    2 +-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 5c4afe6..04802cd 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -32,6 +32,7 @@
 #include <linux/mount.h>
 #include <linux/log2.h>
 #include <linux/quotaops.h>
+#include <linux/genhd.h>
 #include <asm/uaccess.h>
 #include "ext2.h"
 #include "xattr.h"
@@ -68,6 +69,8 @@ void ext2_error (struct super_block * sb, const char * function,
 		printk("Remounting filesystem read-only\n");
 		sb->s_flags |= MS_RDONLY;
 	}
+	if (test_opt(sb, ERR_NOTIFY))
+		notify_part_fs_unclean(part_to_dev(sb->s_bdev->bd_part), 1);
 }
 
 void ext2_warning (struct super_block * sb, const char * function,
@@ -81,6 +84,8 @@ void ext2_warning (struct super_block * sb, const char * function,
 	vprintk(fmt, args);
 	printk("\n");
 	va_end(args);
+	if (test_opt(sb, ERR_NOTIFY))
+		notify_part_fs_unclean(part_to_dev(sb->s_bdev->bd_part), 1);
 }
 
 void ext2_update_dynamic_rev(struct super_block *sb)
@@ -289,6 +294,9 @@ static int ext2_show_options(struct seq_file *seq, struct vfsmount *vfs)
 	if (!test_opt(sb, RESERVATION))
 		seq_puts(seq, ",noreservation");
 
+	if (!test_opt(sb, ERR_NOTIFY))
+		seq_puts(seq, ",notify");
+
 	return 0;
 }
 
@@ -391,7 +399,8 @@ enum {
 	Opt_err_ro, Opt_nouid32, Opt_nocheck, Opt_debug,
 	Opt_oldalloc, Opt_orlov, Opt_nobh, Opt_user_xattr, Opt_nouser_xattr,
 	Opt_acl, Opt_noacl, Opt_xip, Opt_ignore, Opt_err, Opt_quota,
-	Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation
+	Opt_usrquota, Opt_grpquota, Opt_reservation, Opt_noreservation,
+	Opt_err_notify,
 };
 
 static const match_table_t tokens = {
@@ -425,6 +434,7 @@ static const match_table_t tokens = {
 	{Opt_usrquota, "usrquota"},
 	{Opt_reservation, "reservation"},
 	{Opt_noreservation, "noreservation"},
+	{Opt_err_notify, "notify"},
 	{Opt_err, NULL}
 };
 
@@ -565,6 +575,9 @@ static int parse_options (char * options,
 			clear_opt(sbi->s_mount_opt, RESERVATION);
 			printk("reservations OFF\n");
 			break;
+		case Opt_err_notify:
+			set_opt(sbi->s_mount_opt, ERR_NOTIFY);
+			break;
 		case Opt_ignore:
 			break;
 		default:
diff --git a/include/linux/ext2_fs.h b/include/linux/ext2_fs.h
index 121720d..ecec20b 100644
--- a/include/linux/ext2_fs.h
+++ b/include/linux/ext2_fs.h
@@ -347,7 +347,7 @@ struct ext2_inode {
 #define EXT2_MOUNT_USRQUOTA		0x020000  /* user quota */
 #define EXT2_MOUNT_GRPQUOTA		0x040000  /* group quota */
 #define EXT2_MOUNT_RESERVATION		0x080000  /* Preallocation */
-
+#define EXT2_MOUNT_ERR_NOTIFY		0x100000  /* Error notifications */
 
 #define clear_opt(o, opt)		o &= ~EXT2_MOUNT_##opt
 #define set_opt(o, opt)			o |= EXT2_MOUNT_##opt
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] FAT: add 'errors' mount option
  2009-06-01 14:28 ` [PATCH 1/5] FAT: add 'errors' mount option Denis Karpov
  2009-06-01 14:28   ` [PATCH 2/5] FS: filesystem corruption notification Denis Karpov
@ 2009-06-03  2:49   ` OGAWA Hirofumi
  2009-06-03  9:20     ` Denis Karpov
  1 sibling, 1 reply; 12+ messages in thread
From: OGAWA Hirofumi @ 2009-06-03  2:49 UTC (permalink / raw)
  To: Denis Karpov; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Denis Karpov <ext-denis.2.karpov@nokia.com> writes:

> +#define FAT_ERRORS_CONT		0x1
> +#define FAT_ERRORS_PANIC	0x2
> +#define FAT_ERRORS_RO		0x4

I think this should be scalar? (i.e. 1, 2, 3)

>  struct fat_mount_options {
>  	uid_t fs_uid;
>  	gid_t fs_gid;
> @@ -39,7 +43,8 @@ struct fat_mount_options {
>  		 nocase:1,	  /* Does this need case conversion? 0=need case conversion*/
>  		 usefree:1,	  /* Use free_clusters for FAT32 */
>  		 tz_utc:1,	  /* Filesystem timestamps are in UTC */
> -		 rodir:1;	  /* allow ATTR_RO for directory */
> +		 rodir:1,	  /* allow ATTR_RO for directory */
> +		 errors:3;	  /* On error: continue, panic, go ro */
>  };

Please use "unsigned char errors", instead of bit filed. Below of
name_check would have 1 byte hole.

> @@ -834,6 +834,12 @@ static int fat_show_options(struct seq_file *m, struct vfsmount *mnt)
>  		seq_puts(m, ",flush");
>  	if (opts->tz_utc)
>  		seq_puts(m, ",tz=UTC");
> +	if (opts->errors & FAT_ERRORS_CONT)
> +		seq_puts(m, ",errors=continue");
> +	if (opts->errors & FAT_ERRORS_PANIC)
> +		seq_puts(m, ",errors=panic");
> +	if (opts->errors & FAT_ERRORS_RO)
> +		seq_puts(m, ",errors=ro");

Also this should be scalar?

	if (opts->errors == FAT_ERRORS_CONT)
		seq_puts(m, ",errors=continue");
	else if (opts->errors == FAT_ERRORS_PANIC)
		seq_puts(m, ",errors=panic");
	else
		seq_puts(m, ",errors=ro");

>  static const match_table_t fat_tokens = {
> @@ -882,6 +889,9 @@ static const match_table_t fat_tokens = {
>  	{Opt_obsolate, "posix"},
>  	{Opt_flush, "flush"},
>  	{Opt_tz_utc, "tz=UTC"},
> +	{Opt_err_cont, "errors=continue"},
> +	{Opt_err_panic, "errors=panic"},
> +	{Opt_err_ro, "errors=remount-ro"},
>  	{Opt_err, NULL},
>  };
>  static const match_table_t msdos_tokens = {
> @@ -889,6 +899,9 @@ static const match_table_t msdos_tokens = {
>  	{Opt_nodots, "dotsOK=no"},
>  	{Opt_dots, "dots"},
>  	{Opt_dots, "dotsOK=yes"},
> +	{Opt_err_cont, "errors=continue"},
> +	{Opt_err_panic, "errors=panic"},
> +	{Opt_err_ro, "errors=remount-ro"},
>  	{Opt_err, NULL}
>  };
>  static const match_table_t vfat_tokens = {
> @@ -919,6 +932,9 @@ static const match_table_t vfat_tokens = {
>  	{Opt_nonumtail_yes, "nonumtail=true"},
>  	{Opt_nonumtail_yes, "nonumtail"},
>  	{Opt_rodir, "rodir"},
> +	{Opt_err_cont, "errors=continue"},
> +	{Opt_err_panic, "errors=panic"},
> +	{Opt_err_ro, "errors=remount-ro"},
>  	{Opt_err, NULL}
>  };

"fat_tokens" is used for the both of vfat and msdos. So, you don't need
to add those to "msdos/vfat_tokens".

> @@ -1043,6 +1059,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
>  		case Opt_tz_utc:
>  			opts->tz_utc = 1;
>  			break;
> +		case Opt_err_cont:
> +			opts->errors = FAT_ERRORS_CONT;
> +			break;
> +		case Opt_err_panic:
> +			opts->errors = FAT_ERRORS_PANIC;
> +			break;
> +		case Opt_err_ro:
> +			opts->errors = FAT_ERRORS_RO;
> +			break;
>  
>  		/* msdos specific */
>  		case Opt_dots:
> @@ -1128,6 +1153,9 @@ out:
>  		opts->allow_utime = ~opts->fs_dmask & (S_IWGRP | S_IWOTH);
>  	if (opts->unicode_xlate)
>  		opts->utf8 = 0;
> +	/* Default behavior on fs errors - go r/o */
> +	if (!opts->errors)
> +		opts->errors = FAT_ERRORS_RO;

Is there any reason opts->errors isn't initialized with FAT_ERRORS_RO
like other options?

> +	if (sbi->options.errors & FAT_ERRORS_PANIC)
> +		panic("    FAT fs panic from previous error\n");
> +	if ((sbi->options.errors & FAT_ERRORS_RO) &&
> +				!(s->s_flags & MS_RDONLY)) {
>  		s->s_flags |= MS_RDONLY;
>  		printk(KERN_ERR "    File system has been set read-only\n");
>  	}
>  }

Also, scalar?
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] FAT errors, user space notifications
  2009-06-01 14:28 [PATCH 0/5] FAT errors, user space notifications Denis Karpov
  2009-06-01 14:28 ` [PATCH 1/5] FAT: add 'errors' mount option Denis Karpov
@ 2009-06-03  3:08 ` OGAWA Hirofumi
  2009-06-03 11:36   ` Denis Karpov
  1 sibling, 1 reply; 12+ messages in thread
From: OGAWA Hirofumi @ 2009-06-03  3:08 UTC (permalink / raw)
  To: Denis Karpov; +Cc: lkml, linux-fsdevel, adrian.hunter, artem.bityutskiy

Denis Karpov <ext-denis.2.karpov@nokia.com> writes:

> 1. Options for FAT file system behavior on errors (continue, panic, 
>    remount r/o)
>
>    Current FAT behavior is to remount itself read-only on critical errors.
>    Quite often this causes more harm to user space applications than if the
>    error would be ignored - file system suddenly becoming r/o leads to all 
>    kind of surprises from applications (yes, I know applications should be
>    written properly, this is not always the case). 
>
>    'errors' mount option (equivalent to the one in 
>    ext2 fs) offers possibility for user space to specify the desired behavior.
>    Default behavior is still as it was: remount read-only.
>    [PATCH 1]

I can't see why more harm with r/o though, this would be useful for some
people. Please see the comment to this patch.

> 2. Generic mechanism for notifications of user space about file system's 
>    errors/inconsistency on a particular partition using:
>
>      - sysfs entry /sys/block/<bdev>/<part>/fs_unclean
>      - uevent KOBJ_CHANGE, uevent's environment variable FS_UNCLEAN=[0:1]
>
>    User space might want to monitor these notifications (poll2() on sysfs
>    file or udevd's rule for uevent) and fix the fs damage.
>    File system can be marked clean again by writing '0' to the corresponding 
>    'fs_unclean' sysfs file.
>
>    Reason for this feature: doing full scale fsck on a file system 
>    at mounting time (especially residing on a slow and error prone media 
>    such as flash) takes long. Full fsck results e.g. in slow boot times.
>    Alternative approach is to run limited fsck (or none at all) at 
>    mounting/boot time. At run-rime if an fs error is encountered, notify 
>    the user space and expect it to fix the file system.
>    [PATCH 2]

This means you are assuming the fs driver can detect all kind of
corruption?  It is not true. Mounting corrupted fs is dangerous, and the
fs driver might corrupt the another part of fs silently. (e.g. corrupted
pointer to object wouldn't be detected usually. etc.)

Or, limited check and repair on userspace, and other check is going into
fs driver?

> 3. Make FAT and EXT2 file systems use the above mechanism to optionally 
>    notify user space about errors. Implemented as 'notify' mount option.
>    FAT error reporting facilities had to be re-factored in order to 
>    simplify sending error notifications.
>    [PATCH 3,4,5]

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] FAT: add 'errors' mount option
  2009-06-03  2:49   ` [PATCH 1/5] FAT: add 'errors' " OGAWA Hirofumi
@ 2009-06-03  9:20     ` Denis Karpov
  2009-06-04  3:54       ` OGAWA Hirofumi
  0 siblings, 1 reply; 12+ messages in thread
From: Denis Karpov @ 2009-06-03  9:20 UTC (permalink / raw)
  To: ext OGAWA Hirofumi
  Cc: linux-kernel@vger.kernel.org, linux-fsdevel,
	Hunter Adrian (Nokia-D/Helsinki),
	Bityutskiy Artem (Nokia-D/Helsinki)

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]

On Wed, Jun 03, 2009 at 04:49:22AM +0200, ext OGAWA Hirofumi wrote:

Hello, 
 
> > +#define FAT_ERRORS_CONT		0x1
> > +#define FAT_ERRORS_PANIC	0x2
> > +#define FAT_ERRORS_RO		0x4
> 
> I think this should be scalar? (i.e. 1, 2, 3)
Indeed, these flags are mutually exclusive, fixed.

> >  struct fat_mount_options {
> >  	uid_t fs_uid;
> >  	gid_t fs_gid;
> > @@ -39,7 +43,8 @@ struct fat_mount_options {
> >  		 nocase:1,	  /* Does this need case conversion? 0=need case conversion*/
> >  		 usefree:1,	  /* Use free_clusters for FAT32 */
> >  		 tz_utc:1,	  /* Filesystem timestamps are in UTC */
> > -		 rodir:1;	  /* allow ATTR_RO for directory */
> > +		 rodir:1,	  /* allow ATTR_RO for directory */
> > +		 errors:3;	  /* On error: continue, panic, go ro */
> >  };
> 
> Please use "unsigned char errors", instead of bit filed. Below of
> name_check would have 1 byte hole.
Didn't notice the hole, thanks. Fixed.

> "fat_tokens" is used for the both of vfat and msdos. So, you don't need
> to add those to "msdos/vfat_tokens".
Fixed.
 
> Is there any reason opts->errors isn't initialized with FAT_ERRORS_RO
> like other options?
No, fixed.

New patch is attached. Thank you for the comments,

Denis.

[-- Attachment #2: 0001-FAT-add-errors-mount-option.patch --]
[-- Type: text/x-diff, Size: 11313 bytes --]

>From e486815dc3afee3d36fc3da3fb337494e7dce70b Mon Sep 17 00:00:00 2001
From: Denis Karpov <ext-denis.2.karpov@nokia.com>
Date: Wed, 27 May 2009 13:28:24 +0300
Subject: [PATCH] FAT: add 'errors' mount option

On severe errors FAT remounts itself in read-only mode. Allow to
specify FAT fs desired behavior through 'errors' mount option:
panic, continue or remount read-only.

`mount -t [fat|vfat] -o errors=[panic,remount-ro,continue] \
	<bdev> <mount point>`

This is analog to ext2 fs 'errors' mount option.

Signed-off-by: Denis Karpov <ext-denis.2.karpov@nokia.com>
---
 Documentation/filesystems/vfat.txt |    5 +++++
 fs/fat/cache.c                     |    6 +++---
 fs/fat/dir.c                       |    2 +-
 fs/fat/fat.h                       |    7 ++++++-
 fs/fat/fatent.c                    |    4 ++--
 fs/fat/file.c                      |    2 +-
 fs/fat/inode.c                     |   24 ++++++++++++++++++++++--
 fs/fat/misc.c                      |   23 +++++++++++++++--------
 fs/fat/namei_msdos.c               |    2 +-
 fs/fat/namei_vfat.c                |    2 +-
 10 files changed, 57 insertions(+), 20 deletions(-)

diff --git a/Documentation/filesystems/vfat.txt b/Documentation/filesystems/vfat.txt
index 3a5ddc9..76d6286 100644
--- a/Documentation/filesystems/vfat.txt
+++ b/Documentation/filesystems/vfat.txt
@@ -132,6 +132,11 @@ rodir	      -- FAT has the ATTR_RO (read-only) attribute. But on Windows,
 		 If you want to use ATTR_RO as read-only flag even for
 		 the directory, set this option.
 
+errors=panic|continue|remount-ro
+	      -- specify FAT behavior on critical errors: panic, continue
+		 without doing anything or remopunt the partition in
+		 read-only mode (default behavior).
+
 <bool>: 0,1,yes,no,true,false
 
 TODO
diff --git a/fs/fat/cache.c b/fs/fat/cache.c
index b426022..923990e 100644
--- a/fs/fat/cache.c
+++ b/fs/fat/cache.c
@@ -241,7 +241,7 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 	while (*fclus < cluster) {
 		/* prevent the infinite loop of cluster chain */
 		if (*fclus > limit) {
-			fat_fs_panic(sb, "%s: detected the cluster chain loop"
+			fat_fs_error(sb, "%s: detected the cluster chain loop"
 				     " (i_pos %lld)", __func__,
 				     MSDOS_I(inode)->i_pos);
 			nr = -EIO;
@@ -252,7 +252,7 @@ int fat_get_cluster(struct inode *inode, int cluster, int *fclus, int *dclus)
 		if (nr < 0)
 			goto out;
 		else if (nr == FAT_ENT_FREE) {
-			fat_fs_panic(sb, "%s: invalid cluster chain"
+			fat_fs_error(sb, "%s: invalid cluster chain"
 				     " (i_pos %lld)", __func__,
 				     MSDOS_I(inode)->i_pos);
 			nr = -EIO;
@@ -285,7 +285,7 @@ static int fat_bmap_cluster(struct inode *inode, int cluster)
 	if (ret < 0)
 		return ret;
 	else if (ret == FAT_ENT_EOF) {
-		fat_fs_panic(sb, "%s: request beyond EOF (i_pos %lld)",
+		fat_fs_error(sb, "%s: request beyond EOF (i_pos %lld)",
 			     __func__, MSDOS_I(inode)->i_pos);
 		return -EIO;
 	}
diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 3a7f603..7e7924c 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -1334,7 +1334,7 @@ found:
 			goto error_remove;
 		}
 		if (dir->i_size & (sbi->cluster_size - 1)) {
-			fat_fs_panic(sb, "Odd directory size");
+			fat_fs_error(sb, "Odd directory size");
 			dir->i_size = (dir->i_size + sbi->cluster_size - 1)
 				& ~((loff_t)sbi->cluster_size - 1);
 		}
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index ea440d6..ed10896 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -17,6 +17,10 @@
 #define VFAT_SFN_CREATE_WIN95	0x0100 /* emulate win95 rule for create */
 #define VFAT_SFN_CREATE_WINNT	0x0200 /* emulate winnt rule for create */
 
+#define FAT_ERRORS_CONT		1      /* ignore error and continue */
+#define FAT_ERRORS_PANIC	2      /* panic on error */
+#define FAT_ERRORS_RO		3      /* remount r/o on error */
+
 struct fat_mount_options {
 	uid_t fs_uid;
 	gid_t fs_gid;
@@ -26,6 +30,7 @@ struct fat_mount_options {
 	char *iocharset;          /* Charset used for filename input/display */
 	unsigned short shortname; /* flags for shortname display/create rule */
 	unsigned char name_check; /* r = relaxed, n = normal, s = strict */
+	unsigned char errors;	  /* On error: continue, panic, remount-ro */
 	unsigned short allow_utime;/* permission for setting the [am]time */
 	unsigned quiet:1,         /* set = fake successful chmods and chowns */
 		 showexec:1,      /* set = only set x bit for com/exe/bat */
@@ -310,7 +315,7 @@ extern int fat_fill_super(struct super_block *sb, void *data, int silent,
 extern int fat_flush_inodes(struct super_block *sb, struct inode *i1,
 		            struct inode *i2);
 /* fat/misc.c */
-extern void fat_fs_panic(struct super_block *s, const char *fmt, ...)
+extern void fat_fs_error(struct super_block *s, const char *fmt, ...)
 	__attribute__ ((format (printf, 2, 3))) __cold;
 extern void fat_clusters_flush(struct super_block *sb);
 extern int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster);
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index da6eea4..60c31f7 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -345,7 +345,7 @@ int fat_ent_read(struct inode *inode, struct fat_entry *fatent, int entry)
 
 	if (entry < FAT_START_ENT || sbi->max_cluster <= entry) {
 		fatent_brelse(fatent);
-		fat_fs_panic(sb, "invalid access to FAT (entry 0x%08x)", entry);
+		fat_fs_error(sb, "invalid access to FAT (entry 0x%08x)", entry);
 		return -EIO;
 	}
 
@@ -557,7 +557,7 @@ int fat_free_clusters(struct inode *inode, int cluster)
 			err = cluster;
 			goto error;
 		} else if (cluster == FAT_ENT_FREE) {
-			fat_fs_panic(sb, "%s: deleting FAT entry beyond EOF",
+			fat_fs_error(sb, "%s: deleting FAT entry beyond EOF",
 				     __func__);
 			err = -EIO;
 			goto error;
diff --git a/fs/fat/file.c b/fs/fat/file.c
index 0a7f4a9..6214287 100644
--- a/fs/fat/file.c
+++ b/fs/fat/file.c
@@ -213,7 +213,7 @@ static int fat_free(struct inode *inode, int skip)
 			fatent_brelse(&fatent);
 			return 0;
 		} else if (ret == FAT_ENT_FREE) {
-			fat_fs_panic(sb,
+			fat_fs_error(sb,
 				     "%s: invalid cluster chain (i_pos %lld)",
 				     __func__, MSDOS_I(inode)->i_pos);
 			ret = -EIO;
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 296785a..7c7ab3a 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -76,7 +76,7 @@ static inline int __fat_get_block(struct inode *inode, sector_t iblock,
 		return 0;
 
 	if (iblock != MSDOS_I(inode)->mmu_private >> sb->s_blocksize_bits) {
-		fat_fs_panic(sb, "corrupted file size (i_pos %lld, %lld)",
+		fat_fs_error(sb, "corrupted file size (i_pos %lld, %lld)",
 			MSDOS_I(inode)->i_pos, MSDOS_I(inode)->mmu_private);
 		return -EIO;
 	}
@@ -834,6 +834,12 @@ static int fat_show_options(struct seq_file *m, struct vfsmount *mnt)
 		seq_puts(m, ",flush");
 	if (opts->tz_utc)
 		seq_puts(m, ",tz=UTC");
+	if (opts->errors == FAT_ERRORS_CONT)
+		seq_puts(m, ",errors=continue");
+	else if (opts->errors == FAT_ERRORS_PANIC)
+		seq_puts(m, ",errors=panic");
+	else
+		seq_puts(m, ",errors=remount-ro");
 
 	return 0;
 }
@@ -846,7 +852,8 @@ enum {
 	Opt_charset, Opt_shortname_lower, Opt_shortname_win95,
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
-	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err,
+	Opt_obsolate, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
+	Opt_err_panic, Opt_err_ro, Opt_err,
 };
 
 static const match_table_t fat_tokens = {
@@ -882,6 +889,9 @@ static const match_table_t fat_tokens = {
 	{Opt_obsolate, "posix"},
 	{Opt_flush, "flush"},
 	{Opt_tz_utc, "tz=UTC"},
+	{Opt_err_cont, "errors=continue"},
+	{Opt_err_panic, "errors=panic"},
+	{Opt_err_ro, "errors=remount-ro"},
 	{Opt_err, NULL},
 };
 static const match_table_t msdos_tokens = {
@@ -951,6 +961,7 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 	opts->numtail = 1;
 	opts->usefree = opts->nocase = 0;
 	opts->tz_utc = 0;
+	opts->errors = FAT_ERRORS_RO;
 	*debug = 0;
 
 	if (!options)
@@ -1043,6 +1054,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
 		case Opt_tz_utc:
 			opts->tz_utc = 1;
 			break;
+		case Opt_err_cont:
+			opts->errors = FAT_ERRORS_CONT;
+			break;
+		case Opt_err_panic:
+			opts->errors = FAT_ERRORS_PANIC;
+			break;
+		case Opt_err_ro:
+			opts->errors = FAT_ERRORS_RO;
+			break;
 
 		/* msdos specific */
 		case Opt_dots:
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index ac39ebc..42972b5 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -12,14 +12,19 @@
 #include "fat.h"
 
 /*
- * fat_fs_panic reports a severe file system problem and sets the file system
- * read-only. The file system can be made writable again by remounting it.
+ * fat_fs_error reports a file system problem that might indicate fa data
+ * corruption/inconsistency. Depending on 'errors' mount option the
+ * panic() is called, or error message is printed FAT and nothing is done,
+ * or filesystem is remounted read-only (default behavior).
+ * In case the file system is remounted read-only, it can be made writable
+ * again by remounting it.
  */
-void fat_fs_panic(struct super_block *s, const char *fmt, ...)
+void fat_fs_error(struct super_block *s, const char *fmt, ...)
 {
 	va_list args;
+	struct msdos_sb_info *sbi = MSDOS_SB(s);
 
-	printk(KERN_ERR "FAT: Filesystem panic (dev %s)\n", s->s_id);
+	printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
 
 	printk(KERN_ERR "    ");
 	va_start(args, fmt);
@@ -27,13 +32,15 @@ void fat_fs_panic(struct super_block *s, const char *fmt, ...)
 	va_end(args);
 	printk("\n");
 
-	if (!(s->s_flags & MS_RDONLY)) {
+	if (sbi->options.errors == FAT_ERRORS_PANIC)
+		panic("    FAT fs panic from previous error\n");
+	if ((sbi->options.errors == FAT_ERRORS_RO) &&
+				!(s->s_flags & MS_RDONLY)) {
 		s->s_flags |= MS_RDONLY;
 		printk(KERN_ERR "    File system has been set read-only\n");
 	}
 }
-
-EXPORT_SYMBOL_GPL(fat_fs_panic);
+EXPORT_SYMBOL_GPL(fat_fs_error);
 
 /* Flushes the number of free clusters on FAT32 */
 /* XXX: Need to write one per FSINFO block.  Currently only writes 1 */
@@ -124,7 +131,7 @@ int fat_chain_add(struct inode *inode, int new_dclus, int nr_cluster)
 			mark_inode_dirty(inode);
 	}
 	if (new_fclus != (inode->i_blocks >> (sbi->cluster_bits - 9))) {
-		fat_fs_panic(sb, "clusters badly computed (%d != %llu)",
+		fat_fs_error(sb, "clusters badly computed (%d != %llu)",
 			     new_fclus,
 			     (llu)(inode->i_blocks >> (sbi->cluster_bits - 9)));
 		fat_cache_inval_inode(inode);
diff --git a/fs/fat/namei_msdos.c b/fs/fat/namei_msdos.c
index da3f361..72f5c64 100644
--- a/fs/fat/namei_msdos.c
+++ b/fs/fat/namei_msdos.c
@@ -608,7 +608,7 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_panic(new_dir->i_sb,
+		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, sinfo.i_pos);
 	}
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index a0e00e3..cb6ddb8 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -1030,7 +1030,7 @@ error_inode:
 		sinfo.bh = NULL;
 	}
 	if (corrupt < 0) {
-		fat_fs_panic(new_dir->i_sb,
+		fat_fs_error(new_dir->i_sb,
 			     "%s: Filesystem corrupted (i_pos %lld)",
 			     __func__, sinfo.i_pos);
 	}
-- 
1.6.3.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] FAT errors, user space notifications
  2009-06-03  3:08 ` [PATCH 0/5] FAT errors, user space notifications OGAWA Hirofumi
@ 2009-06-03 11:36   ` Denis Karpov
  2009-06-03 15:13     ` OGAWA Hirofumi
  0 siblings, 1 reply; 12+ messages in thread
From: Denis Karpov @ 2009-06-03 11:36 UTC (permalink / raw)
  To: ext OGAWA Hirofumi
  Cc: linux-kernel, linux-fsdevel, Hunter Adrian (Nokia-D/Helsinki),
	Bityutskiy Artem (Nokia-D/Helsinki)

On Wed, Jun 03, 2009 at 05:08:10AM +0200, ext OGAWA Hirofumi wrote:
> Denis Karpov <ext-denis.2.karpov@nokia.com> writes:
> 
> > 1. Options for FAT file system behavior on errors (continue, panic, 
> >    remount r/o)
> >
> >    Current FAT behavior is to remount itself read-only on critical errors.
> >    Quite often this causes more harm to user space applications than if the
> >    error would be ignored - file system suddenly becoming r/o leads to all 
> >    kind of surprises from applications (yes, I know applications should be
> >    written properly, this is not always the case). 
> >
> >    'errors' mount option (equivalent to the one in 
> >    ext2 fs) offers possibility for user space to specify the desired behavior.
> >    Default behavior is still as it was: remount read-only.
> >    [PATCH 1]
> 
> I can't see why more harm with r/o though, this would be useful for some
> people. 

Not 'harm' really, but not a nice thing either - for an user space application
having open fds or pwd on a partition that has become read-only. Anyway,
the default behavior is unchanged and alternatives are optional.

> Please see the comment to this patch.
Thank you for the review, fixed according to comments.

> > 2. Generic mechanism for notifications of user space about file system's 
> >    errors/inconsistency on a particular partition using:
> >
> >      - sysfs entry /sys/block/<bdev>/<part>/fs_unclean
> >      - uevent KOBJ_CHANGE, uevent's environment variable FS_UNCLEAN=[0:1]
> >
> >    User space might want to monitor these notifications (poll2() on sysfs
> >    file or udevd's rule for uevent) and fix the fs damage.
> >    File system can be marked clean again by writing '0' to the corresponding 
> >    'fs_unclean' sysfs file.
> >
> >    Reason for this feature: doing full scale fsck on a file system 
> >    at mounting time (especially residing on a slow and error prone media 
> >    such as flash) takes long. Full fsck results e.g. in slow boot times.
> >    Alternative approach is to run limited fsck (or none at all) at 
> >    mounting/boot time. At run-rime if an fs error is encountered, notify 
> >    the user space and expect it to fix the file system.
> >    [PATCH 2]
> 
> This means you are assuming the fs driver can detect all kind of
> corruption?  It is not true. Mounting corrupted fs is dangerous, and the
> fs driver might corrupt the another part of fs silently. (e.g. corrupted
> pointer to object wouldn't be detected usually. etc.)

I realise that, but in this particular case I deal with non-critical data 
on a large FAT partition and can probably afford certain risk of damaging
the data. What I can't afford is to spend several minutes fsck'ing huge FAT
partition on slow SD/MMC media during bootup.

So I choose to optionally receive notification of errors encountered 
during 'run time' and act upon them.

Otherwise, nothing stops you from doing proper fsck before mounting.

IMO, receivng notification of errors is benefitial in any case:
together with the 1st patch above it gives full flexibility to user space
to implement fs 'run-time' errors handling policy (at least for FAT,EXT2),
e.g.:

- do nothing: remount r/o on errors, don't monitor kernel notifications (old/default
 behavior)
- remount-ro on errors, get notified; unmount partition, fsck, mount
  partition back r/w;
- ignore errors (continue), get notified: unmount the partition later at
suitable time, fsck, mount back r/w
 
> Or, limited check and repair on userspace, and other check is going into
> fs driver?
> 
> > 3. Make FAT and EXT2 file systems use the above mechanism to optionally 
> >    notify user space about errors. Implemented as 'notify' mount option.
> >    FAT error reporting facilities had to be re-factored in order to 
> >    simplify sending error notifications.
> >    [PATCH 3,4,5]
> 
> Thanks.

'user space notification' patches 2-5 above need a bit more work, I'll resend
them.

best regards,
Denis

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/5] FAT errors, user space notifications
  2009-06-03 11:36   ` Denis Karpov
@ 2009-06-03 15:13     ` OGAWA Hirofumi
  0 siblings, 0 replies; 12+ messages in thread
From: OGAWA Hirofumi @ 2009-06-03 15:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fsdevel, Hunter Adrian (Nokia-D/Helsinki),
	Bityutskiy Artem (Nokia-D/Helsinki)

Denis Karpov <ext-denis.2.karpov@nokia.com> writes:

> I realise that, but in this particular case I deal with non-critical data 
> on a large FAT partition and can probably afford certain risk of damaging
> the data. What I can't afford is to spend several minutes fsck'ing huge FAT
> partition on slow SD/MMC media during bootup.
>
> So I choose to optionally receive notification of errors encountered 
> during 'run time' and act upon them.
>
> Otherwise, nothing stops you from doing proper fsck before mounting.

I think fsckless is to add the reliability to fs driver (logging,
softupdate, etc.). Yes, it's not easy, and it needs time. Anyway, I
actually thought about softupdate (and some others) before, I think it's
_not_ nothing.

> IMO, receivng notification of errors is benefitial in any case:
> together with the 1st patch above it gives full flexibility to user space
> to implement fs 'run-time' errors handling policy (at least for FAT,EXT2),
> e.g.:
>
> - do nothing: remount r/o on errors, don't monitor kernel notifications (old/default
>  behavior)
> - remount-ro on errors, get notified; unmount partition, fsck, mount
>   partition back r/w;
> - ignore errors (continue), get notified: unmount the partition later at
> suitable time, fsck, mount back r/w

If this is monitoring interface, I guess it should be more generic. And
I guess it will tell what happened in kernel, not fs_clean. (There is no
guarantee about fs state)

If not, some errors can not be detected by fs driver. User may know some
run-time errors by fs_clean, but some run-time errors is not. So, user
can not trust fs_clean.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/5] FAT: add 'errors' mount option
  2009-06-03  9:20     ` Denis Karpov
@ 2009-06-04  3:54       ` OGAWA Hirofumi
  0 siblings, 0 replies; 12+ messages in thread
From: OGAWA Hirofumi @ 2009-06-04  3:54 UTC (permalink / raw)
  To: Denis Karpov
  Cc: linux-kernel@vger.kernel.org, linux-fsdevel,
	Hunter Adrian (Nokia-D/Helsinki),
	Bityutskiy Artem (Nokia-D/Helsinki)

Denis Karpov <ext-denis.2.karpov@nokia.com> writes:

> New patch is attached. Thank you for the comments,

Thanks. I've applied this patch.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-06-04  3:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-01 14:28 [PATCH 0/5] FAT errors, user space notifications Denis Karpov
2009-06-01 14:28 ` [PATCH 1/5] FAT: add 'errors' mount option Denis Karpov
2009-06-01 14:28   ` [PATCH 2/5] FS: filesystem corruption notification Denis Karpov
2009-06-01 14:28     ` [PATCH 3/5] FAT: generalize errors and warning printing Denis Karpov
2009-06-01 14:28       ` [PATCH 4/5] FAT: add 'notify' mount option Denis Karpov
2009-06-01 14:28         ` [PATCH 5/5] EXT2: " Denis Karpov
2009-06-03  2:49   ` [PATCH 1/5] FAT: add 'errors' " OGAWA Hirofumi
2009-06-03  9:20     ` Denis Karpov
2009-06-04  3:54       ` OGAWA Hirofumi
2009-06-03  3:08 ` [PATCH 0/5] FAT errors, user space notifications OGAWA Hirofumi
2009-06-03 11:36   ` Denis Karpov
2009-06-03 15:13     ` OGAWA Hirofumi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).