2010-11-09 16:55:35

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 0/2] FAT unified kernel messages

Hi all,
i do not know best place to send patches for FAT fs, so i send it here.
This patchset add function fat_msg and remplace all printk to use it.
The major differnce is - it add device name to all messages.

For example old log:
FAT: im filed.
new one:
FAT-fs (sda1): im filed.

Second reson is, to make FAT logs more grep frendly. So you can grep by "-fs"
for (EXT[4,3,2] and FAT), or by device.


2010-11-09 16:55:42

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 1/2] Introduce fat_msg() for unified kernel messages

Add fat_msg() function to unify printkas. And use it
to report mounts and remounts.

new dmesg looks like this:
[ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
[ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/inode.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index ad6998a..4699173 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -37,6 +37,20 @@
static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET;

+/* unify messages.
+ * this function is copy of ext4_msg() from fs/ext4/super.c
+ */
+void fat_msg(struct super_block *sb, const char *prefix,
+ const char *fmt, ...)
+{
+ va_list args;
+
+ va_start(args, fmt);
+ printk("%sFAT-fs (%s): ", prefix, sb->s_id);
+ vprintk(fmt, args);
+ printk("\n");
+ va_end(args);
+}

static int fat_add_cluster(struct inode *inode)
{
@@ -552,6 +566,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
{
struct msdos_sb_info *sbi = MSDOS_SB(sb);
*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
+
+ fat_msg(sb, KERN_INFO, "re-mounted. Opts: %s", data);
return 0;
}

@@ -1249,6 +1265,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
unsigned int media;
long error;
char buf[50];
+ char *orig_data = kstrdup(data, GFP_KERNEL);

/*
* GFP_KERNEL is ok here, because while we do hold the
@@ -1502,6 +1519,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
goto out_fail;
}

+ fat_msg(sb, KERN_INFO, "Mounted. Opts: %s", orig_data);
+ kfree(orig_data);
+
return 0;

out_invalid:
--
1.7.1

2010-11-09 16:55:50

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 2/2] Unify rest of FAT messages.

Here is how it looks like after patch.

[50334.635174] FAT-fs (loop0): utf8 is not a recommended IO charset for FAT filesystems, filesystem will be case sensitive!
[50334.635178]
[50334.635606] FAT-fs (loop0): Invalid FSINFO signature: 0x00000000, 0x00000000 (sector = 1)
[50334.635666] FAT-fs (loop0): Mounted. Opts: iocharset=utf8
[50342.129047] FAT-fs (loop0): error, invalid access to FAT (entry 0x9e90a346)
[50342.129306] FAT-fs (loop0): error, invalid access to FAT (entry 0xa35ced48)

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/dir.c | 9 ++++---
fs/fat/fat.h | 2 +
fs/fat/fatent.c | 4 +-
fs/fat/inode.c | 68 +++++++++++++++++++++++++++---------------------------
fs/fat/misc.c | 14 +++++------
5 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index ee42b9e..4d1d9e3 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -98,7 +98,7 @@ next:

*bh = sb_bread(sb, phys);
if (*bh == NULL) {
- printk(KERN_ERR "FAT: Directory bread(block %llu) failed\n",
+ fat_msg(sb, KERN_ERR, "Directory bread(block %llu) failed",
(llu)phys);
/* skip this block */
*pos = (iblock + 1) << sb->s_blocksize_bits;
@@ -979,6 +979,7 @@ static int __fat_remove_entries(struct inode *dir, loff_t pos, int nr_slots)

int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
{
+ struct super_block *sb = dir->i_sb;
struct msdos_dir_entry *de;
struct buffer_head *bh;
int err = 0, nr_slots;
@@ -1013,8 +1014,8 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
*/
err = __fat_remove_entries(dir, sinfo->slot_off, nr_slots);
if (err) {
- printk(KERN_WARNING
- "FAT: Couldn't remove the long name slots\n");
+ fat_msg(sb, KERN_WARNING,
+ "Couldn't remove the long name slots");
}
}

@@ -1265,7 +1266,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "Corrupted directory (i_pos %lld)",
MSDOS_I(dir)->i_pos);
err = -EIO;
goto error;
diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index d75a77f..e945916 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -345,4 +345,6 @@ void fat_cache_destroy(void);
/* helper for printk */
typedef unsigned long long llu;

+void fat_msg(struct super_block *sb, const char *prefix, const char *fmt, ...);
+
#endif /* !_FAT_H */
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index b47d2c9..2e81ac0 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -95,7 +95,7 @@ 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_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)", (llu)blocknr);
return -EIO;
}

@@ -108,7 +108,7 @@ static int fat_ent_bread(struct super_block *sb, struct fat_entry *fatent,
fatent->fat_inode = MSDOS_SB(sb)->fat_inode;
fatent->bhs[0] = sb_bread(sb, blocknr);
if (!fatent->bhs[0]) {
- printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n",
+ fat_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)",
(llu)blocknr);
return -EIO;
}
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index 4699173..6e08a48 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -629,8 +629,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_msg(sb, KERN_ERR, "unable to read inode block "
+ "for updating (i_pos %lld)", i_pos);
return -EIO;
}
spin_lock(&sbi->inode_hash_lock);
@@ -990,8 +990,8 @@ static const match_table_t vfat_tokens = {
{Opt_err, NULL}
};

-static int parse_options(char *options, int is_vfat, int silent, int *debug,
- struct fat_mount_options *opts)
+static int parse_options(struct super_block *sb, char *options, int is_vfat,
+ int silent, int *debug, struct fat_mount_options *opts)
{
char *p;
substring_t args[MAX_OPT_ARGS];
@@ -1182,15 +1182,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,

/* obsolete mount options */
case Opt_obsolate:
- printk(KERN_INFO "FAT: \"%s\" option is obsolete, "
- "not supported now\n", p);
+ fat_msg(sb, KERN_INFO, "\"%s\" option is obsolete, "
+ "not supported now", p);
break;
/* unknown option */
default:
if (!silent) {
- printk(KERN_ERR
- "FAT: Unrecognized mount option \"%s\" "
- "or missing value\n", p);
+ fat_msg(sb, KERN_ERR,
+ "Unrecognized mount option \"%s\" "
+ "or missing value", p);
}
return -EINVAL;
}
@@ -1199,7 +1199,7 @@ 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"
+ fat_msg(sb, KERN_ERR, "utf8 is not a recommended IO charset"
" for FAT filesystems, filesystem will be "
"case sensitive!\n");
}
@@ -1286,7 +1286,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);

- error = parse_options(data, isvfat, silent, &debug, &sbi->options);
+ error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
if (error)
goto out_fail;

@@ -1294,20 +1294,21 @@ 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_msg(sb, KERN_ERR, "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_msg(sb,
+ KERN_ERR, "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_msg(sb, KERN_ERR, "bogus number of FAT structure");
brelse(bh);
goto out_invalid;
}
@@ -1320,7 +1321,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
media);
brelse(bh);
goto out_invalid;
@@ -1330,7 +1331,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
logical_sector_size);
brelse(bh);
goto out_invalid;
@@ -1338,15 +1339,15 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
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",
+ fat_msg(sb, KERN_ERR, "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_msg(sb, KERN_ERR, "logical sector size too small for device"
+ " (logical sector size = %u)", logical_sector_size);
brelse(bh);
goto out_fail;
}
@@ -1354,14 +1355,14 @@ 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",
+ fat_msg(sb, KERN_ERR, "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",
+ fat_msg(sb, KERN_ERR, "unable to read boot sector"
+ " (logical sector size = %lu)",
sb->s_blocksize);
goto out_fail;
}
@@ -1397,16 +1398,16 @@ 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_msg(sb, KERN_ERR, "bread failed, FSINFO block"
+ " (sector = %lu)", sbi->fsinfo_sector);
brelse(bh);
goto out_fail;
}

fsinfo = (struct fat_boot_fsinfo *)fsinfo_bh->b_data;
if (!IS_FSINFO(fsinfo)) {
- printk(KERN_WARNING "FAT: Invalid FSINFO signature: "
- "0x%08x, 0x%08x (sector = %lu)\n",
+ fat_msg(sb, KERN_WARNING, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
@@ -1427,8 +1428,8 @@ 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_msg(sb, KERN_ERR, "bogus directroy-entries per block"
+ " (%u)", sbi->dir_entries);
brelse(bh);
goto out_invalid;
}
@@ -1450,7 +1451,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "count of clusters too big (%u)",
total_clusters);
brelse(bh);
goto out_invalid;
@@ -1483,7 +1484,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_msg(sb, KERN_ERR, "codepage %s not found", buf);
goto out_fail;
}

@@ -1491,7 +1492,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "IO charset %s not found",
sbi->options.iocharset);
goto out_fail;
}
@@ -1515,7 +1516,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_msg(sb, KERN_ERR, "get root inode failed");
goto out_fail;
}

@@ -1527,8 +1528,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
out_invalid:
error = -EINVAL;
if (!silent)
- printk(KERN_INFO "VFS: Can't find a valid FAT filesystem"
- " on dev %s.\n", sb->s_id);
+ fat_msg(sb, KERN_INFO, "Can't find a valid FAT filesystem");

out_fail:
if (fat_inode)
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 970e682..8e043a1 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -26,9 +26,7 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
va_list args;

if (report) {
- printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
-
- printk(KERN_ERR " ");
+ printk(KERN_ERR "FAT-fs (%s): error, ", s->s_id);
va_start(args, fmt);
vprintk(fmt, args);
va_end(args);
@@ -36,10 +34,10 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
}

if (opts->errors == FAT_ERRORS_PANIC)
- panic("FAT: fs panic from previous error\n");
+ panic("FAT-fs (%s): fs panic from previous error\n", s->s_id);
else if (opts->errors == FAT_ERRORS_RO && !(s->s_flags & MS_RDONLY)) {
s->s_flags |= MS_RDONLY;
- printk(KERN_ERR "FAT: Filesystem has been set read-only\n");
+ fat_msg(s, KERN_ERR, "Filesystem has been set read-only");
}
}
EXPORT_SYMBOL_GPL(__fat_fs_error);
@@ -57,15 +55,15 @@ int 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_msg(sb, KERN_ERR, "bread failed in fat_clusters_flush");
return -EIO;
}

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_msg(sb, KERN_ERR, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
--
1.7.1

2010-11-09 17:03:30

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 0/2] FAT unified kernel messages

On Tue, 9 Nov 2010 17:55:16 +0100 Alexey Fisher wrote:

> Hi all,
> i do not know best place to send patches for FAT fs, so i send it here.

This mailing list is fine, but you should also Cc: the maintainer as listed
in the MAINTAINERS file:

OGAWA Hirofumi <[email protected]>


> This patchset add function fat_msg and remplace all printk to use it.
> The major differnce is - it add device name to all messages.
>
> For example old log:
> FAT: im filed.
> new one:
> FAT-fs (sda1): im filed.
>
> Second reson is, to make FAT logs more grep frendly. So you can grep by "-fs"
> for (EXT[4,3,2] and FAT), or by device.


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2010-11-09 17:25:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] Introduce fat_msg() for unified kernel messages

On Tue, 2010-11-09 at 17:55 +0100, Alexey Fisher wrote:
> Add fat_msg() function to unify printkas. And use it
> to report mounts and remounts.

Hi Alexey.

> new dmesg looks like this:
> [ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
> [ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)
>
> Signed-off-by: Alexey Fisher <[email protected]>
> ---
> fs/fat/inode.c | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index ad6998a..4699173 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -37,6 +37,20 @@
> static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
> static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET;
>
> +/* unify messages.
> + * this function is copy of ext4_msg() from fs/ext4/super.c
> + */
> +void fat_msg(struct super_block *sb, const char *prefix,
> + const char *fmt, ...)
> +{
> + va_list args;
> +
> + va_start(args, fmt);
> + printk("%sFAT-fs (%s): ", prefix, sb->s_id);
> + vprintk(fmt, args);
> + printk("\n");
> + va_end(args);
> +}

A few comments:

The first patch should put the prototype for fat_msg in
fs/fat/fat.h file and the body as done here.

The prototype should use attribute_ printf so format
and argument are type checked.

This would be better using the %pV extension so a
single printk call is done and the dmesg log can not be
interleaved.

prefix is a bit misleading, perhaps level is better.

void __attribute__ ((format (printf, 3, 4)))
fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
{
struct va_format vaf;
va_list args;

va_start(args, fmt);
vaf.fmt = fmt;
vaf.args = &args;

printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);

va_end(args);
}

2010-11-09 17:56:21

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 1/2] Introduce fat_msg() for unified kernel messages

Hi Joe,

On Tue, Nov 09, 2010 at 09:25:41AM -0800, Joe Perches wrote:
> On Tue, 2010-11-09 at 17:55 +0100, Alexey Fisher wrote:
> > Add fat_msg() function to unify printkas. And use it
> > to report mounts and remounts.
>
> Hi Alexey.
>
> > new dmesg looks like this:
> > [ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
> > [ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)
> >
> > Signed-off-by: Alexey Fisher <[email protected]>
> > ---
> > fs/fat/inode.c | 20 ++++++++++++++++++++
> > 1 files changed, 20 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> > index ad6998a..4699173 100644
> > --- a/fs/fat/inode.c
> > +++ b/fs/fat/inode.c
> > @@ -37,6 +37,20 @@
> > static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
> > static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET;
> >
> > +/* unify messages.
> > + * this function is copy of ext4_msg() from fs/ext4/super.c
> > + */
> > +void fat_msg(struct super_block *sb, const char *prefix,
> > + const char *fmt, ...)
> > +{
> > + va_list args;
> > +
> > + va_start(args, fmt);
> > + printk("%sFAT-fs (%s): ", prefix, sb->s_id);
> > + vprintk(fmt, args);
> > + printk("\n");
> > + va_end(args);
> > +}
>
> A few comments:
>
> The first patch should put the prototype for fat_msg in
> fs/fat/fat.h file and the body as done here.
>
> The prototype should use attribute_ printf so format
> and argument are type checked.
>
> This would be better using the %pV extension so a
> single printk call is done and the dmesg log can not be
> interleaved.
>
> prefix is a bit misleading, perhaps level is better.

Note that this routine was almost 1:1 taken from ext2, it seems (which is a
sensible approach), so these issues are present more than once.

Regards,

Wolfram

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (1.98 kB)
signature.asc (197.00 B)
Digital signature
Download all attachments

2010-11-09 18:59:20

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 1/2] Idd fat_msg() to unified kernel messages for FAT fs.

Add fat_msg() function to unify printkas. And use it
to report mounts and remounts.

new dmesg looks like this:
[ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
[ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)

v2 - add prototype to fat.h;
úse %pV; rename prefix to level;

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/fat.h | 2 ++
fs/fat/inode.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index d75a77f..c9e4553 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -345,4 +345,6 @@ void fat_cache_destroy(void);
/* helper for printk */
typedef unsigned long long llu;

+void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
+
#endif /* !_FAT_H */
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index ad6998a..a4e8f26 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -37,6 +37,24 @@
static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET;

+/**
+ * fat_msg() - print preformated FAT specific messages.
+ * this function is copy of ext4_msg() from fs/ext4/super.c
+ */
+void __attribute__ ((format (printf, 3, 4)))
+fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
+
+ va_end(args);
+}

static int fat_add_cluster(struct inode *inode)
{
@@ -552,6 +570,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
{
struct msdos_sb_info *sbi = MSDOS_SB(sb);
*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
+
+ fat_msg(sb, KERN_INFO, "re-mounted. Opts: %s", data);
return 0;
}

@@ -1249,6 +1269,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
unsigned int media;
long error;
char buf[50];
+ char *orig_data = kstrdup(data, GFP_KERNEL);

/*
* GFP_KERNEL is ok here, because while we do hold the
@@ -1502,6 +1523,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
goto out_fail;
}

+ fat_msg(sb, KERN_INFO, "Mounted. Opts: %s", orig_data);
+ kfree(orig_data);
+
return 0;

out_invalid:
--
1.7.1

2010-11-09 19:04:41

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 2/2 v2] Unify rest of FAT messages.

Here is how it looks like after patch.

[50334.635174] FAT-fs (loop0): utf8 is not a recommended IO charset for FAT filesystems, filesystem will be case sensitive!
[50334.635178]
[50334.635606] FAT-fs (loop0): Invalid FSINFO signature: 0x00000000, 0x00000000 (sector = 1)
[50334.635666] FAT-fs (loop0): Mounted. Opts: iocharset=utf8
[50342.129047] FAT-fs (loop0): error, invalid access to FAT (entry 0x9e90a346)
[50342.129306] FAT-fs (loop0): error, invalid access to FAT (entry 0xa35ced48)

v2 - more protoype to other patch

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/dir.c | 9 ++++---
fs/fat/fatent.c | 4 +-
fs/fat/inode.c | 68 +++++++++++++++++++++++++++---------------------------
fs/fat/misc.c | 14 +++++------
4 files changed, 47 insertions(+), 48 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index ee42b9e..4d1d9e3 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -98,7 +98,7 @@ next:

*bh = sb_bread(sb, phys);
if (*bh == NULL) {
- printk(KERN_ERR "FAT: Directory bread(block %llu) failed\n",
+ fat_msg(sb, KERN_ERR, "Directory bread(block %llu) failed",
(llu)phys);
/* skip this block */
*pos = (iblock + 1) << sb->s_blocksize_bits;
@@ -979,6 +979,7 @@ static int __fat_remove_entries(struct inode *dir, loff_t pos, int nr_slots)

int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
{
+ struct super_block *sb = dir->i_sb;
struct msdos_dir_entry *de;
struct buffer_head *bh;
int err = 0, nr_slots;
@@ -1013,8 +1014,8 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
*/
err = __fat_remove_entries(dir, sinfo->slot_off, nr_slots);
if (err) {
- printk(KERN_WARNING
- "FAT: Couldn't remove the long name slots\n");
+ fat_msg(sb, KERN_WARNING,
+ "Couldn't remove the long name slots");
}
}

@@ -1265,7 +1266,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "Corrupted directory (i_pos %lld)",
MSDOS_I(dir)->i_pos);
err = -EIO;
goto error;
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index b47d2c9..2e81ac0 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -95,7 +95,7 @@ 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_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)", (llu)blocknr);
return -EIO;
}

@@ -108,7 +108,7 @@ static int fat_ent_bread(struct super_block *sb, struct fat_entry *fatent,
fatent->fat_inode = MSDOS_SB(sb)->fat_inode;
fatent->bhs[0] = sb_bread(sb, blocknr);
if (!fatent->bhs[0]) {
- printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n",
+ fat_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)",
(llu)blocknr);
return -EIO;
}
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index a4e8f26..f799b00 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -633,8 +633,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_msg(sb, KERN_ERR, "unable to read inode block "
+ "for updating (i_pos %lld)", i_pos);
return -EIO;
}
spin_lock(&sbi->inode_hash_lock);
@@ -994,8 +994,8 @@ static const match_table_t vfat_tokens = {
{Opt_err, NULL}
};

-static int parse_options(char *options, int is_vfat, int silent, int *debug,
- struct fat_mount_options *opts)
+static int parse_options(struct super_block *sb, char *options, int is_vfat,
+ int silent, int *debug, struct fat_mount_options *opts)
{
char *p;
substring_t args[MAX_OPT_ARGS];
@@ -1186,15 +1186,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,

/* obsolete mount options */
case Opt_obsolate:
- printk(KERN_INFO "FAT: \"%s\" option is obsolete, "
- "not supported now\n", p);
+ fat_msg(sb, KERN_INFO, "\"%s\" option is obsolete, "
+ "not supported now", p);
break;
/* unknown option */
default:
if (!silent) {
- printk(KERN_ERR
- "FAT: Unrecognized mount option \"%s\" "
- "or missing value\n", p);
+ fat_msg(sb, KERN_ERR,
+ "Unrecognized mount option \"%s\" "
+ "or missing value", p);
}
return -EINVAL;
}
@@ -1203,7 +1203,7 @@ 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"
+ fat_msg(sb, KERN_ERR, "utf8 is not a recommended IO charset"
" for FAT filesystems, filesystem will be "
"case sensitive!\n");
}
@@ -1290,7 +1290,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);

- error = parse_options(data, isvfat, silent, &debug, &sbi->options);
+ error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
if (error)
goto out_fail;

@@ -1298,20 +1298,21 @@ 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_msg(sb, KERN_ERR, "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_msg(sb,
+ KERN_ERR, "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_msg(sb, KERN_ERR, "bogus number of FAT structure");
brelse(bh);
goto out_invalid;
}
@@ -1324,7 +1325,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
media);
brelse(bh);
goto out_invalid;
@@ -1334,7 +1335,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
logical_sector_size);
brelse(bh);
goto out_invalid;
@@ -1342,15 +1343,15 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
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",
+ fat_msg(sb, KERN_ERR, "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_msg(sb, KERN_ERR, "logical sector size too small for device"
+ " (logical sector size = %u)", logical_sector_size);
brelse(bh);
goto out_fail;
}
@@ -1358,14 +1359,14 @@ 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",
+ fat_msg(sb, KERN_ERR, "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",
+ fat_msg(sb, KERN_ERR, "unable to read boot sector"
+ " (logical sector size = %lu)",
sb->s_blocksize);
goto out_fail;
}
@@ -1401,16 +1402,16 @@ 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_msg(sb, KERN_ERR, "bread failed, FSINFO block"
+ " (sector = %lu)", sbi->fsinfo_sector);
brelse(bh);
goto out_fail;
}

fsinfo = (struct fat_boot_fsinfo *)fsinfo_bh->b_data;
if (!IS_FSINFO(fsinfo)) {
- printk(KERN_WARNING "FAT: Invalid FSINFO signature: "
- "0x%08x, 0x%08x (sector = %lu)\n",
+ fat_msg(sb, KERN_WARNING, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
@@ -1431,8 +1432,8 @@ 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_msg(sb, KERN_ERR, "bogus directroy-entries per block"
+ " (%u)", sbi->dir_entries);
brelse(bh);
goto out_invalid;
}
@@ -1454,7 +1455,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "count of clusters too big (%u)",
total_clusters);
brelse(bh);
goto out_invalid;
@@ -1487,7 +1488,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_msg(sb, KERN_ERR, "codepage %s not found", buf);
goto out_fail;
}

@@ -1495,7 +1496,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "IO charset %s not found",
sbi->options.iocharset);
goto out_fail;
}
@@ -1519,7 +1520,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_msg(sb, KERN_ERR, "get root inode failed");
goto out_fail;
}

@@ -1531,8 +1532,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
out_invalid:
error = -EINVAL;
if (!silent)
- printk(KERN_INFO "VFS: Can't find a valid FAT filesystem"
- " on dev %s.\n", sb->s_id);
+ fat_msg(sb, KERN_INFO, "Can't find a valid FAT filesystem");

out_fail:
if (fat_inode)
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 970e682..8e043a1 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -26,9 +26,7 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
va_list args;

if (report) {
- printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
-
- printk(KERN_ERR " ");
+ printk(KERN_ERR "FAT-fs (%s): error, ", s->s_id);
va_start(args, fmt);
vprintk(fmt, args);
va_end(args);
@@ -36,10 +34,10 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
}

if (opts->errors == FAT_ERRORS_PANIC)
- panic("FAT: fs panic from previous error\n");
+ panic("FAT-fs (%s): fs panic from previous error\n", s->s_id);
else if (opts->errors == FAT_ERRORS_RO && !(s->s_flags & MS_RDONLY)) {
s->s_flags |= MS_RDONLY;
- printk(KERN_ERR "FAT: Filesystem has been set read-only\n");
+ fat_msg(s, KERN_ERR, "Filesystem has been set read-only");
}
}
EXPORT_SYMBOL_GPL(__fat_fs_error);
@@ -57,15 +55,15 @@ int 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_msg(sb, KERN_ERR, "bread failed in fat_clusters_flush");
return -EIO;
}

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_msg(sb, KERN_ERR, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
--
1.7.1

2010-11-09 19:43:28

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] Idd fat_msg() to unified kernel messages for FAT fs.

On Tue, 2010-11-09 at 19:58 +0100, Alexey Fisher wrote:
> Add fat_msg() function to unify printkas. And use it
> to report mounts and remounts.
>
> new dmesg looks like this:
> [ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
> [ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)
>
> v2 - add prototype to fat.h;
> úse %pV; rename prefix to level;

Hi again Alexey.

I think putting this function in misc.c is better
because __fat_fs_error is already in that file.

Patch 1 should just add the prototype and function
and maybe convert __fat_fs_error to use %pV.

Maybe the __fat_fs_error %pV conversion is a
separate patch.

The last patch should do the conversions to
fat_msg.

Perhaps add this to misc.c:

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 970e682..b497918 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -24,15 +24,18 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
{
struct fat_mount_options *opts = &MSDOS_SB(s)->options;
va_list args;
+ struct va_format vaf;

if (report) {
- printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
-
- printk(KERN_ERR " ");
va_start(args, fmt);
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR "FAT: Filesystem error (dev %s): %pV\n",
+ s->s_id, &vaf);
+
va_end(args);
- printk("\n");
}

if (opts->errors == FAT_ERRORS_PANIC)


2010-11-10 10:23:26

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 1/4] Convert fat_fs_error to use %pV

- convert fat_fs_error to use %pV
- be consequent and use "supor_block *sb" instead of "supor_block *s"
- use devise name in each message.

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/fat.h | 10 +++++-----
fs/fat/misc.c | 25 ++++++++++++++-----------
2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index d75a77f..4f99235 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -325,12 +325,12 @@ 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, int report, const char *fmt, ...)
+__fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
__attribute__ ((format (printf, 3, 4))) __cold;
-#define fat_fs_error(s, fmt, args...) \
- __fat_fs_error(s, 1, fmt , ## args)
-#define fat_fs_error_ratelimit(s, fmt, args...) \
- __fat_fs_error(s, __ratelimit(&MSDOS_SB(s)->ratelimit), fmt , ## args)
+#define fat_fs_error(sb, fmt, args...) \
+ __fat_fs_error(sb, 1, fmt , ## args)
+#define fat_fs_error_ratelimit(sb, fmt, args...) \
+ __fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
extern int 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/misc.c b/fs/fat/misc.c
index 970e682..958c5f9 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -20,26 +20,29 @@
* 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, int report, const char *fmt, ...)
+void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
{
- struct fat_mount_options *opts = &MSDOS_SB(s)->options;
+ struct fat_mount_options *opts = &MSDOS_SB(sb)->options;
va_list args;
+ struct va_format vaf;

if (report) {
- printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
-
- printk(KERN_ERR " ");
va_start(args, fmt);
- vprintk(fmt, args);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk(KERN_ERR "FAT-fs (%s): error, %pV\n", sb->s_id, &vaf);
+
va_end(args);
- printk("\n");
}

if (opts->errors == FAT_ERRORS_PANIC)
- panic("FAT: fs panic from previous error\n");
- else if (opts->errors == FAT_ERRORS_RO && !(s->s_flags & MS_RDONLY)) {
- s->s_flags |= MS_RDONLY;
- printk(KERN_ERR "FAT: Filesystem has been set read-only\n");
+ panic("FAT-fs (%s): fs panic from previous error\n", sb->s_id);
+ else if (opts->errors == FAT_ERRORS_RO && !(sb->s_flags & MS_RDONLY)) {
+ sb->s_flags |= MS_RDONLY;
+ printk(KERN_ERR "FAT-fs (%s): Filesystem has been "
+ "set read-only\n", sb->s_id);
}
}
EXPORT_SYMBOL_GPL(__fat_fs_error);
--
1.7.1

2010-11-10 10:23:35

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 4/4] Report each FAT mount and mount options.

Report each mount and mount options. Example:
FAT-fs (sdc1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/inode.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index a85adf8..75d936f 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -552,6 +552,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
{
struct msdos_sb_info *sbi = MSDOS_SB(sb);
*flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
+
+ fat_msg(sb, KERN_INFO, "re-mounted. Opts: %s", data);
return 0;
}

@@ -1249,6 +1251,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
unsigned int media;
long error;
char buf[50];
+ char *orig_data = kstrdup(data, GFP_KERNEL);

/*
* GFP_KERNEL is ok here, because while we do hold the
@@ -1503,6 +1506,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
goto out_fail;
}

+ fat_msg(sb, KERN_INFO, "Mounted. Opts: %s", orig_data);
+ kfree(orig_data);
+
return 0;

out_invalid:
--
1.7.1

2010-11-10 10:23:50

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 3/4] Replace all printk with fat_msg()

Replace all printk with fat_msg()

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/dir.c | 9 ++++---
fs/fat/fatent.c | 4 +-
fs/fat/inode.c | 68 +++++++++++++++++++++++++++---------------------------
fs/fat/misc.c | 6 ++--
4 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index ee42b9e..4d1d9e3 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -98,7 +98,7 @@ next:

*bh = sb_bread(sb, phys);
if (*bh == NULL) {
- printk(KERN_ERR "FAT: Directory bread(block %llu) failed\n",
+ fat_msg(sb, KERN_ERR, "Directory bread(block %llu) failed",
(llu)phys);
/* skip this block */
*pos = (iblock + 1) << sb->s_blocksize_bits;
@@ -979,6 +979,7 @@ static int __fat_remove_entries(struct inode *dir, loff_t pos, int nr_slots)

int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
{
+ struct super_block *sb = dir->i_sb;
struct msdos_dir_entry *de;
struct buffer_head *bh;
int err = 0, nr_slots;
@@ -1013,8 +1014,8 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
*/
err = __fat_remove_entries(dir, sinfo->slot_off, nr_slots);
if (err) {
- printk(KERN_WARNING
- "FAT: Couldn't remove the long name slots\n");
+ fat_msg(sb, KERN_WARNING,
+ "Couldn't remove the long name slots");
}
}

@@ -1265,7 +1266,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "Corrupted directory (i_pos %lld)",
MSDOS_I(dir)->i_pos);
err = -EIO;
goto error;
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index b47d2c9..2e81ac0 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -95,7 +95,7 @@ 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_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)", (llu)blocknr);
return -EIO;
}

@@ -108,7 +108,7 @@ static int fat_ent_bread(struct super_block *sb, struct fat_entry *fatent,
fatent->fat_inode = MSDOS_SB(sb)->fat_inode;
fatent->bhs[0] = sb_bread(sb, blocknr);
if (!fatent->bhs[0]) {
- printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n",
+ fat_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)",
(llu)blocknr);
return -EIO;
}
diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index ad6998a..a85adf8 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -613,8 +613,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_msg(sb, KERN_ERR, "unable to read inode block "
+ "for updating (i_pos %lld)", i_pos);
return -EIO;
}
spin_lock(&sbi->inode_hash_lock);
@@ -974,8 +974,8 @@ static const match_table_t vfat_tokens = {
{Opt_err, NULL}
};

-static int parse_options(char *options, int is_vfat, int silent, int *debug,
- struct fat_mount_options *opts)
+static int parse_options(struct super_block *sb, char *options, int is_vfat,
+ int silent, int *debug, struct fat_mount_options *opts)
{
char *p;
substring_t args[MAX_OPT_ARGS];
@@ -1166,15 +1166,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,

/* obsolete mount options */
case Opt_obsolate:
- printk(KERN_INFO "FAT: \"%s\" option is obsolete, "
- "not supported now\n", p);
+ fat_msg(sb, KERN_INFO, "\"%s\" option is obsolete, "
+ "not supported now", p);
break;
/* unknown option */
default:
if (!silent) {
- printk(KERN_ERR
- "FAT: Unrecognized mount option \"%s\" "
- "or missing value\n", p);
+ fat_msg(sb, KERN_ERR,
+ "Unrecognized mount option \"%s\" "
+ "or missing value", p);
}
return -EINVAL;
}
@@ -1183,7 +1183,7 @@ 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"
+ fat_msg(sb, KERN_ERR, "utf8 is not a recommended IO charset"
" for FAT filesystems, filesystem will be "
"case sensitive!\n");
}
@@ -1269,7 +1269,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
DEFAULT_RATELIMIT_BURST);

- error = parse_options(data, isvfat, silent, &debug, &sbi->options);
+ error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
if (error)
goto out_fail;

@@ -1277,20 +1277,21 @@ 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_msg(sb, KERN_ERR, "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_msg(sb,
+ KERN_ERR, "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_msg(sb, KERN_ERR, "bogus number of FAT structure");
brelse(bh);
goto out_invalid;
}
@@ -1303,7 +1304,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
media);
brelse(bh);
goto out_invalid;
@@ -1313,7 +1314,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
logical_sector_size);
brelse(bh);
goto out_invalid;
@@ -1321,15 +1322,15 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
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",
+ fat_msg(sb, KERN_ERR, "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_msg(sb, KERN_ERR, "logical sector size too small for device"
+ " (logical sector size = %u)", logical_sector_size);
brelse(bh);
goto out_fail;
}
@@ -1337,14 +1338,14 @@ 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",
+ fat_msg(sb, KERN_ERR, "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",
+ fat_msg(sb, KERN_ERR, "unable to read boot sector"
+ " (logical sector size = %lu)",
sb->s_blocksize);
goto out_fail;
}
@@ -1380,16 +1381,16 @@ 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_msg(sb, KERN_ERR, "bread failed, FSINFO block"
+ " (sector = %lu)", sbi->fsinfo_sector);
brelse(bh);
goto out_fail;
}

fsinfo = (struct fat_boot_fsinfo *)fsinfo_bh->b_data;
if (!IS_FSINFO(fsinfo)) {
- printk(KERN_WARNING "FAT: Invalid FSINFO signature: "
- "0x%08x, 0x%08x (sector = %lu)\n",
+ fat_msg(sb, KERN_WARNING, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
@@ -1410,8 +1411,8 @@ 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_msg(sb, KERN_ERR, "bogus directroy-entries per block"
+ " (%u)", sbi->dir_entries);
brelse(bh);
goto out_invalid;
}
@@ -1433,7 +1434,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "count of clusters too big (%u)",
total_clusters);
brelse(bh);
goto out_invalid;
@@ -1466,7 +1467,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_msg(sb, KERN_ERR, "codepage %s not found", buf);
goto out_fail;
}

@@ -1474,7 +1475,7 @@ 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",
+ fat_msg(sb, KERN_ERR, "IO charset %s not found",
sbi->options.iocharset);
goto out_fail;
}
@@ -1498,7 +1499,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_msg(sb, KERN_ERR, "get root inode failed");
goto out_fail;
}

@@ -1507,8 +1508,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
out_invalid:
error = -EINVAL;
if (!silent)
- printk(KERN_INFO "VFS: Can't find a valid FAT filesystem"
- " on dev %s.\n", sb->s_id);
+ fat_msg(sb, KERN_INFO, "Can't find a valid FAT filesystem");

out_fail:
if (fat_inode)
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index 3aac24b..8dbe48d 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -79,15 +79,15 @@ int 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_msg(sb, KERN_ERR, "bread failed in fat_clusters_flush");
return -EIO;
}

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_msg(sb, KERN_ERR, "Invalid FSINFO signature: "
+ "0x%08x, 0x%08x (sector = %lu)",
le32_to_cpu(fsinfo->signature1),
le32_to_cpu(fsinfo->signature2),
sbi->fsinfo_sector);
--
1.7.1

2010-11-10 10:23:30

by Oleksij Rempel

[permalink] [raw]
Subject: [PATCH 2/4] Add fat_msg() function for preformated FAT messages

Add fat_msg() to replace not cosequent used printk() in fs/fat/*
New message format should be as fallow:
FAT-fs (sda1): some thing happened.

Signed-off-by: Alexey Fisher <[email protected]>
---
fs/fat/fat.h | 1 +
fs/fat/misc.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 4f99235..803eade 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -331,6 +331,7 @@ __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
__fat_fs_error(sb, 1, fmt , ## args)
#define fat_fs_error_ratelimit(sb, fmt, args...) \
__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
+void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
extern int 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/misc.c b/fs/fat/misc.c
index 958c5f9..3aac24b 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -47,6 +47,25 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...)
}
EXPORT_SYMBOL_GPL(__fat_fs_error);

+/**
+ * fat_msg() - print preformated FAT specific messages. Every thing what is
+ * not fat_fs_error() should be fat_msg().
+ */
+void __attribute__ ((format (printf, 3, 4)))
+fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
+
+ va_end(args);
+}
+
/* Flushes the number of free clusters on FAT32 */
/* XXX: Need to write one per FSINFO block. Currently only writes 1 */
int fat_clusters_flush(struct super_block *sb)
--
1.7.1

2010-11-10 10:32:46

by Andreas Schwab

[permalink] [raw]
Subject: Re: [PATCH 1/4] Convert fat_fs_error to use %pV

Alexey Fisher <[email protected]> writes:

> - be consequent and use "supor_block *sb" instead of "supor_block *s"

s/supor/super/

Andreas.

--
Andreas Schwab, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."

2010-11-10 11:40:11

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Alexey Fisher <[email protected]> writes:

> Here is how it looks like after patch.

This is going to unify the all FSes (e.g. fat_msg() uses fs_msg("FAT", ...))?
If so, I think it's welcome. If not, umm, it looks like good but...

Thanks.

> [50334.635174] FAT-fs (loop0): utf8 is not a recommended IO charset for FAT filesystems, filesystem will be case sensitive!
> [50334.635178]
> [50334.635606] FAT-fs (loop0): Invalid FSINFO signature: 0x00000000, 0x00000000 (sector = 1)
> [50334.635666] FAT-fs (loop0): Mounted. Opts: iocharset=utf8
> [50342.129047] FAT-fs (loop0): error, invalid access to FAT (entry 0x9e90a346)
> [50342.129306] FAT-fs (loop0): error, invalid access to FAT (entry 0xa35ced48)
>
> v2 - more protoype to other patch
>
> Signed-off-by: Alexey Fisher <[email protected]>
> ---
> fs/fat/dir.c | 9 ++++---
> fs/fat/fatent.c | 4 +-
> fs/fat/inode.c | 68 +++++++++++++++++++++++++++---------------------------
> fs/fat/misc.c | 14 +++++------
> 4 files changed, 47 insertions(+), 48 deletions(-)
>
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index ee42b9e..4d1d9e3 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -98,7 +98,7 @@ next:
>
> *bh = sb_bread(sb, phys);
> if (*bh == NULL) {
> - printk(KERN_ERR "FAT: Directory bread(block %llu) failed\n",
> + fat_msg(sb, KERN_ERR, "Directory bread(block %llu) failed",
> (llu)phys);
> /* skip this block */
> *pos = (iblock + 1) << sb->s_blocksize_bits;
> @@ -979,6 +979,7 @@ static int __fat_remove_entries(struct inode *dir, loff_t pos, int nr_slots)
>
> int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
> {
> + struct super_block *sb = dir->i_sb;
> struct msdos_dir_entry *de;
> struct buffer_head *bh;
> int err = 0, nr_slots;
> @@ -1013,8 +1014,8 @@ int fat_remove_entries(struct inode *dir, struct fat_slot_info *sinfo)
> */
> err = __fat_remove_entries(dir, sinfo->slot_off, nr_slots);
> if (err) {
> - printk(KERN_WARNING
> - "FAT: Couldn't remove the long name slots\n");
> + fat_msg(sb, KERN_WARNING,
> + "Couldn't remove the long name slots");
> }
> }
>
> @@ -1265,7 +1266,7 @@ 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",
> + fat_msg(sb, KERN_ERR, "Corrupted directory (i_pos %lld)",
> MSDOS_I(dir)->i_pos);
> err = -EIO;
> goto error;
> diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
> index b47d2c9..2e81ac0 100644
> --- a/fs/fat/fatent.c
> +++ b/fs/fat/fatent.c
> @@ -95,7 +95,7 @@ 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_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)", (llu)blocknr);
> return -EIO;
> }
>
> @@ -108,7 +108,7 @@ static int fat_ent_bread(struct super_block *sb, struct fat_entry *fatent,
> fatent->fat_inode = MSDOS_SB(sb)->fat_inode;
> fatent->bhs[0] = sb_bread(sb, blocknr);
> if (!fatent->bhs[0]) {
> - printk(KERN_ERR "FAT: FAT read failed (blocknr %llu)\n",
> + fat_msg(sb, KERN_ERR, "FAT read failed (blocknr %llu)",
> (llu)blocknr);
> return -EIO;
> }
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index a4e8f26..f799b00 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -633,8 +633,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_msg(sb, KERN_ERR, "unable to read inode block "
> + "for updating (i_pos %lld)", i_pos);
> return -EIO;
> }
> spin_lock(&sbi->inode_hash_lock);
> @@ -994,8 +994,8 @@ static const match_table_t vfat_tokens = {
> {Opt_err, NULL}
> };
>
> -static int parse_options(char *options, int is_vfat, int silent, int *debug,
> - struct fat_mount_options *opts)
> +static int parse_options(struct super_block *sb, char *options, int is_vfat,
> + int silent, int *debug, struct fat_mount_options *opts)
> {
> char *p;
> substring_t args[MAX_OPT_ARGS];
> @@ -1186,15 +1186,15 @@ static int parse_options(char *options, int is_vfat, int silent, int *debug,
>
> /* obsolete mount options */
> case Opt_obsolate:
> - printk(KERN_INFO "FAT: \"%s\" option is obsolete, "
> - "not supported now\n", p);
> + fat_msg(sb, KERN_INFO, "\"%s\" option is obsolete, "
> + "not supported now", p);
> break;
> /* unknown option */
> default:
> if (!silent) {
> - printk(KERN_ERR
> - "FAT: Unrecognized mount option \"%s\" "
> - "or missing value\n", p);
> + fat_msg(sb, KERN_ERR,
> + "Unrecognized mount option \"%s\" "
> + "or missing value", p);
> }
> return -EINVAL;
> }
> @@ -1203,7 +1203,7 @@ 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"
> + fat_msg(sb, KERN_ERR, "utf8 is not a recommended IO charset"
> " for FAT filesystems, filesystem will be "
> "case sensitive!\n");
> }
> @@ -1290,7 +1290,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
> ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL,
> DEFAULT_RATELIMIT_BURST);
>
> - error = parse_options(data, isvfat, silent, &debug, &sbi->options);
> + error = parse_options(sb, data, isvfat, silent, &debug, &sbi->options);
> if (error)
> goto out_fail;
>
> @@ -1298,20 +1298,21 @@ 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_msg(sb, KERN_ERR, "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_msg(sb,
> + KERN_ERR, "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_msg(sb, KERN_ERR, "bogus number of FAT structure");
> brelse(bh);
> goto out_invalid;
> }
> @@ -1324,7 +1325,7 @@ 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",
> + fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",
> media);
> brelse(bh);
> goto out_invalid;
> @@ -1334,7 +1335,7 @@ 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",
> + fat_msg(sb, KERN_ERR, "bogus logical sector size %u",
> logical_sector_size);
> brelse(bh);
> goto out_invalid;
> @@ -1342,15 +1343,15 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
> 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",
> + fat_msg(sb, KERN_ERR, "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_msg(sb, KERN_ERR, "logical sector size too small for device"
> + " (logical sector size = %u)", logical_sector_size);
> brelse(bh);
> goto out_fail;
> }
> @@ -1358,14 +1359,14 @@ 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",
> + fat_msg(sb, KERN_ERR, "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",
> + fat_msg(sb, KERN_ERR, "unable to read boot sector"
> + " (logical sector size = %lu)",
> sb->s_blocksize);
> goto out_fail;
> }
> @@ -1401,16 +1402,16 @@ 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_msg(sb, KERN_ERR, "bread failed, FSINFO block"
> + " (sector = %lu)", sbi->fsinfo_sector);
> brelse(bh);
> goto out_fail;
> }
>
> fsinfo = (struct fat_boot_fsinfo *)fsinfo_bh->b_data;
> if (!IS_FSINFO(fsinfo)) {
> - printk(KERN_WARNING "FAT: Invalid FSINFO signature: "
> - "0x%08x, 0x%08x (sector = %lu)\n",
> + fat_msg(sb, KERN_WARNING, "Invalid FSINFO signature: "
> + "0x%08x, 0x%08x (sector = %lu)",
> le32_to_cpu(fsinfo->signature1),
> le32_to_cpu(fsinfo->signature2),
> sbi->fsinfo_sector);
> @@ -1431,8 +1432,8 @@ 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_msg(sb, KERN_ERR, "bogus directroy-entries per block"
> + " (%u)", sbi->dir_entries);
> brelse(bh);
> goto out_invalid;
> }
> @@ -1454,7 +1455,7 @@ 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",
> + fat_msg(sb, KERN_ERR, "count of clusters too big (%u)",
> total_clusters);
> brelse(bh);
> goto out_invalid;
> @@ -1487,7 +1488,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_msg(sb, KERN_ERR, "codepage %s not found", buf);
> goto out_fail;
> }
>
> @@ -1495,7 +1496,7 @@ 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",
> + fat_msg(sb, KERN_ERR, "IO charset %s not found",
> sbi->options.iocharset);
> goto out_fail;
> }
> @@ -1519,7 +1520,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_msg(sb, KERN_ERR, "get root inode failed");
> goto out_fail;
> }
>
> @@ -1531,8 +1532,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
> out_invalid:
> error = -EINVAL;
> if (!silent)
> - printk(KERN_INFO "VFS: Can't find a valid FAT filesystem"
> - " on dev %s.\n", sb->s_id);
> + fat_msg(sb, KERN_INFO, "Can't find a valid FAT filesystem");
>
> out_fail:
> if (fat_inode)
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index 970e682..8e043a1 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -26,9 +26,7 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
> va_list args;
>
> if (report) {
> - printk(KERN_ERR "FAT: Filesystem error (dev %s)\n", s->s_id);
> -
> - printk(KERN_ERR " ");
> + printk(KERN_ERR "FAT-fs (%s): error, ", s->s_id);
> va_start(args, fmt);
> vprintk(fmt, args);
> va_end(args);
> @@ -36,10 +34,10 @@ void __fat_fs_error(struct super_block *s, int report, const char *fmt, ...)
> }
>
> if (opts->errors == FAT_ERRORS_PANIC)
> - panic("FAT: fs panic from previous error\n");
> + panic("FAT-fs (%s): fs panic from previous error\n", s->s_id);
> else if (opts->errors == FAT_ERRORS_RO && !(s->s_flags & MS_RDONLY)) {
> s->s_flags |= MS_RDONLY;
> - printk(KERN_ERR "FAT: Filesystem has been set read-only\n");
> + fat_msg(s, KERN_ERR, "Filesystem has been set read-only");
> }
> }
> EXPORT_SYMBOL_GPL(__fat_fs_error);
> @@ -57,15 +55,15 @@ int 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_msg(sb, KERN_ERR, "bread failed in fat_clusters_flush");
> return -EIO;
> }
>
> 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_msg(sb, KERN_ERR, "Invalid FSINFO signature: "
> + "0x%08x, 0x%08x (sector = %lu)",
> le32_to_cpu(fsinfo->signature1),
> le32_to_cpu(fsinfo->signature2),
> sbi->fsinfo_sector);

--
OGAWA Hirofumi <[email protected]>

2010-11-10 11:58:21

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 1/2] Idd fat_msg() to unified kernel messages for FAT fs.

Alexey Fisher <[email protected]> writes:

> Add fat_msg() function to unify printkas. And use it
> to report mounts and remounts.

Sorry, I missed to see the intent of this. What's for?

Passed options and accepted options are not same, IMO, so reporting
passed option are confusable. No?

Thanks.

> new dmesg looks like this:
> [ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush
> [ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null)
>
> v2 - add prototype to fat.h;
> $BC:(Bse %pV; rename prefix to level;
>
> Signed-off-by: Alexey Fisher <[email protected]>
> ---
> fs/fat/fat.h | 2 ++
> fs/fat/inode.c | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+), 0 deletions(-)
>
> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index d75a77f..c9e4553 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -345,4 +345,6 @@ void fat_cache_destroy(void);
> /* helper for printk */
> typedef unsigned long long llu;
>
> +void fat_msg(struct super_block *sb, const char *level, const char *fmt, ...);
> +
> #endif /* !_FAT_H */
> diff --git a/fs/fat/inode.c b/fs/fat/inode.c
> index ad6998a..a4e8f26 100644
> --- a/fs/fat/inode.c
> +++ b/fs/fat/inode.c
> @@ -37,6 +37,24 @@
> static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE;
> static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET;
>
> +/**
> + * fat_msg() - print preformated FAT specific messages.
> + * this function is copy of ext4_msg() from fs/ext4/super.c
> + */
> +void __attribute__ ((format (printf, 3, 4)))
> +fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, fmt);
> + vaf.fmt = fmt;
> + vaf.va = &args;
> +
> + printk("%sFAT-fs (%s): %pV\n", level, sb->s_id, &vaf);
> +
> + va_end(args);
> +}
>
> static int fat_add_cluster(struct inode *inode)
> {
> @@ -552,6 +570,8 @@ static int fat_remount(struct super_block *sb, int *flags, char *data)
> {
> struct msdos_sb_info *sbi = MSDOS_SB(sb);
> *flags |= MS_NODIRATIME | (sbi->options.isvfat ? 0 : MS_NOATIME);
> +
> + fat_msg(sb, KERN_INFO, "re-mounted. Opts: %s", data);
> return 0;
> }
>
> @@ -1249,6 +1269,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
> unsigned int media;
> long error;
> char buf[50];
> + char *orig_data = kstrdup(data, GFP_KERNEL);
>
> /*
> * GFP_KERNEL is ok here, because while we do hold the
> @@ -1502,6 +1523,9 @@ int fat_fill_super(struct super_block *sb, void *data, int silent,
> goto out_fail;
> }
>
> + fat_msg(sb, KERN_INFO, "Mounted. Opts: %s", orig_data);
> + kfree(orig_data);
> +
> return 0;
>
> out_invalid:

--
OGAWA Hirofumi <[email protected]>

2010-11-10 12:41:53

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Am Mittwoch, den 10.11.2010, 20:40 +0900 schrieb OGAWA Hirofumi:
> Alexey Fisher <[email protected]> writes:
>
> > Here is how it looks like after patch.
>
> This is going to unify the all FSes (e.g. fat_msg() uses fs_msg("FAT", ...))?
> If so, I think it's welcome. If not, umm, it looks like good but...

Yea, i thought about it. It will be better to have some kernel wide
fs_msg(). But before doing this i think one more step should be done -
some notification interface for userspace.
If we get some fs errors userspace app should be notified so user too.

I work currently on one case, user lost all data on FAT drive. First
error messages was logged for some weeks, and only now some big data
lost. There war no errors from GUI.

---
Regards,
Alexey

2010-11-10 13:53:19

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Alexey Fisher <[email protected]> writes:

> Am Mittwoch, den 10.11.2010, 20:40 +0900 schrieb OGAWA Hirofumi:
>> Alexey Fisher <[email protected]> writes:
>>
>> > Here is how it looks like after patch.
>>
>> This is going to unify the all FSes (e.g. fat_msg() uses fs_msg("FAT", ...))?
>> If so, I think it's welcome. If not, umm, it looks like good but...
>
> Yea, i thought about it. It will be better to have some kernel wide
> fs_msg(). But before doing this i think one more step should be done -
> some notification interface for userspace.
> If we get some fs errors userspace app should be notified so user too.
>
> I work currently on one case, user lost all data on FAT drive. First
> error messages was logged for some weeks, and only now some big data
> lost. There war no errors from GUI.

It clealy can happen on all FSes, not only FAT. And I don't want to push
it (notification stuff) from FAT tree. (need the review from other guys).

IMHO, it should be kernel wide (or subsystem wide) notification system
even if you target is FAT-fs only. (yeah, it's hard to push to
linus-tree than one FS though).

Thanks.
--
OGAWA Hirofumi <[email protected]>

2010-11-10 15:07:28

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Am Mittwoch, den 10.11.2010, 22:53 +0900 schrieb OGAWA Hirofumi:
> Alexey Fisher <[email protected]> writes:
>
> > Am Mittwoch, den 10.11.2010, 20:40 +0900 schrieb OGAWA Hirofumi:
> >> Alexey Fisher <[email protected]> writes:
> >>
> >> > Here is how it looks like after patch.
> >>
> >> This is going to unify the all FSes (e.g. fat_msg() uses fs_msg("FAT", ...))?
> >> If so, I think it's welcome. If not, umm, it looks like good but...
> >
> > Yea, i thought about it. It will be better to have some kernel wide
> > fs_msg(). But before doing this i think one more step should be done -
> > some notification interface for userspace.
> > If we get some fs errors userspace app should be notified so user too.
> >
> > I work currently on one case, user lost all data on FAT drive. First
> > error messages was logged for some weeks, and only now some big data
> > lost. There war no errors from GUI.
>
> It clealy can happen on all FSes, not only FAT. And I don't want to push
> it (notification stuff) from FAT tree. (need the review from other guys).

I do not blame FAT. It was just one example/usecase.

> IMHO, it should be kernel wide (or subsystem wide) notification system
> even if you target is FAT-fs only. (yeah, it's hard to push to
> linus-tree than one FS though).

Sound complicated. I will not get time for this, right now. Any way, at
least if this patch set go upstream it will help investigate some FS
related issues. For my work (Computer Forensic) it will be really
helpful.

Regards,
Alexey

2010-11-10 16:41:06

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Alexey Fisher <[email protected]> writes:

>> It clealy can happen on all FSes, not only FAT. And I don't want to push
>> it (notification stuff) from FAT tree. (need the review from other guys).
>
> I do not blame FAT. It was just one example/usecase.

Yes. It is why I said the below.

>> IMHO, it should be kernel wide (or subsystem wide) notification system
>> even if you target is FAT-fs only. (yeah, it's hard to push to
>> linus-tree than one FS though).
>
> Sound complicated. I will not get time for this, right now. Any way, at
> least if this patch set go upstream it will help investigate some FS
> related issues. For my work (Computer Forensic) it will be really
> helpful.

Also, yes. It would be complicate more or less than one FS. And maybe I
can understand why you want to concentrate FAT.

But I'm afraid to implement for only FAT. If there is not any some sort
of agreement by others, it may have to re-implement. If that happen, FAT
can be bothered by compatible (or have to maintain both of old and new
for a while). Because the notified data are some sort of ABI, IMO.

This is why I'm careful about this, and can't say easily - "let's try it
on FAT".

Thanks.
--
OGAWA Hirofumi <[email protected]>

2010-11-10 16:58:39

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Am Donnerstag, den 11.11.2010, 01:40 +0900 schrieb OGAWA Hirofumi:
> Alexey Fisher <[email protected]> writes:
>
> >> It clealy can happen on all FSes, not only FAT. And I don't want to push
> >> it (notification stuff) from FAT tree. (need the review from other guys).
> >
> > I do not blame FAT. It was just one example/usecase.
>
> Yes. It is why I said the below.
>
> >> IMHO, it should be kernel wide (or subsystem wide) notification system
> >> even if you target is FAT-fs only. (yeah, it's hard to push to
> >> linus-tree than one FS though).
> >
> > Sound complicated. I will not get time for this, right now. Any way, at
> > least if this patch set go upstream it will help investigate some FS
> > related issues. For my work (Computer Forensic) it will be really
> > helpful.
>
> Also, yes. It would be complicate more or less than one FS. And maybe I
> can understand why you want to concentrate FAT.
>
> But I'm afraid to implement for only FAT. If there is not any some sort
> of agreement by others, it may have to re-implement. If that happen, FAT
> can be bothered by compatible (or have to maintain both of old and new
> for a while). Because the notified data are some sort of ABI, IMO.
>
> This is why I'm careful about this, and can't say easily - "let's try it
> on FAT".

I'm not sure if we talk about same thing now :D
By notification you mean the idea to notify userspace about error?
correct?
In my previous email i switched the point, it will be to complicate for
now to work on notification system. For now i'll be happy to see grep
friendly logs. Same like ext2,3,4 do

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4cf46b67eb6de94532c1bea11d2479d085229d0e

In long term it will be good to have such notification system. So i will
start new tread about it.

2010-11-10 20:53:39

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Alexey Fisher <[email protected]> writes:

>> Also, yes. It would be complicate more or less than one FS. And maybe I
>> can understand why you want to concentrate FAT.
>>
>> But I'm afraid to implement for only FAT. If there is not any some sort
>> of agreement by others, it may have to re-implement. If that happen, FAT
>> can be bothered by compatible (or have to maintain both of old and new
>> for a while). Because the notified data are some sort of ABI, IMO.
>>
>> This is why I'm careful about this, and can't say easily - "let's try it
>> on FAT".
>
> I'm not sure if we talk about same thing now :D
> By notification you mean the idea to notify userspace about error?
> correct?

Sure. I was talking about "to notify userspace". :)

> In my previous email i switched the point, it will be to complicate for
> now to work on notification system. For now i'll be happy to see grep
> friendly logs. Same like ext2,3,4 do
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4cf46b67eb6de94532c1bea11d2479d085229d0e
>
> In long term it will be good to have such notification system. So i will
> start new tread about it.

OK, I see. I'll review patches. Looks good though.

Thanks.
--
OGAWA Hirofumi <[email protected]>

2011-02-24 08:05:00

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Hi,

are there any updates on this issue?

Am Donnerstag, den 11.11.2010, 05:53 +0900 schrieb OGAWA Hirofumi:
> Alexey Fisher <[email protected]> writes:
>
> >> Also, yes. It would be complicate more or less than one FS. And maybe I
> >> can understand why you want to concentrate FAT.
> >>
> >> But I'm afraid to implement for only FAT. If there is not any some sort
> >> of agreement by others, it may have to re-implement. If that happen, FAT
> >> can be bothered by compatible (or have to maintain both of old and new
> >> for a while). Because the notified data are some sort of ABI, IMO.
> >>
> >> This is why I'm careful about this, and can't say easily - "let's try it
> >> on FAT".
> >
> > I'm not sure if we talk about same thing now :D
> > By notification you mean the idea to notify userspace about error?
> > correct?
>
> Sure. I was talking about "to notify userspace". :)
>
> > In my previous email i switched the point, it will be to complicate for
> > now to work on notification system. For now i'll be happy to see grep
> > friendly logs. Same like ext2,3,4 do
> >
> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4cf46b67eb6de94532c1bea11d2479d085229d0e
> >
> > In long term it will be good to have such notification system. So i will
> > start new tread about it.
>
> OK, I see. I'll review patches. Looks good though.
>
> Thanks.

2011-03-02 14:37:38

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] Unify rest of FAT messages.

Alexey Fisher <[email protected]> writes:

> Hi,
>
> are there any updates on this issue?

Sorry for late reply. Honestly I'm not reviewing those yet. But I'll
apply 1/4, 2/4, 3/4 probably.

Thanks.

> Am Donnerstag, den 11.11.2010, 05:53 +0900 schrieb OGAWA Hirofumi:
>> Alexey Fisher <[email protected]> writes:
>>
>> >> Also, yes. It would be complicate more or less than one FS. And maybe I
>> >> can understand why you want to concentrate FAT.
>> >>
>> >> But I'm afraid to implement for only FAT. If there is not any some sort
>> >> of agreement by others, it may have to re-implement. If that happen, FAT
>> >> can be bothered by compatible (or have to maintain both of old and new
>> >> for a while). Because the notified data are some sort of ABI, IMO.
>> >>
>> >> This is why I'm careful about this, and can't say easily - "let's try it
>> >> on FAT".
>> >
>> > I'm not sure if we talk about same thing now :D
>> > By notification you mean the idea to notify userspace about error?
>> > correct?
>>
>> Sure. I was talking about "to notify userspace". :)
>>
>> > In my previous email i switched the point, it will be to complicate for
>> > now to work on notification system. For now i'll be happy to see grep
>> > friendly logs. Same like ext2,3,4 do
>> >
>> > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4cf46b67eb6de94532c1bea11d2479d085229d0e
>> >
>> > In long term it will be good to have such notification system. So i will
>> > start new tread about it.
>>
>> OK, I see. I'll review patches. Looks good though.
>>
>> Thanks.
>
>

--
OGAWA Hirofumi <[email protected]>