fat: print s_dev via fat_msg

[PATCH] fat: print s_dev via fat_msg

Posted by Sungjong Seo 1 year, 5 months ago

To clarify MAJOR/MINOR number of a mounted device, fat_msg prints prefix
that includes them.

Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
---
 fs/fat/fat.h  | 2 +-
 fs/fat/misc.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/fat/fat.h b/fs/fat/fat.h
index 66cf4778cf3b..538bcb3e28e1 100644
--- a/fs/fat/fat.h
+++ b/fs/fat/fat.h
@@ -435,7 +435,7 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...);
 #define fat_fs_error_ratelimit(sb, fmt, args...) \
 	__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
 
-#define FAT_PRINTK_PREFIX "%sFAT-fs (%s): "
+#define FAT_PRINTK_PREFIX "%sFAT-fs (%s[%d:%d]): "
 #define fat_msg(sb, level, fmt, args...)				\
 do {									\
 	printk_index_subsys_emit(FAT_PRINTK_PREFIX, level, fmt, ##args);\
diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index c7a2d27120ba..6672cefc5484 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
 
 #include "fat.h"
 #include <linux/iversion.h>
+#include <linux/blkdev.h>
 
 /*
  * fat_fs_error reports a file system problem that might indicate fa data
@@ -59,7 +60,8 @@ void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
 	va_start(args, fmt);
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id, &vaf);
+	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id,
+		MAJOR(sb->s_dev), MINOR(sb->s_dev), &vaf);
 	va_end(args);
 }
 
-- 
2.25.1

Re: [PATCH] fat: print s_dev via fat_msg

Posted by OGAWA Hirofumi 1 year, 5 months ago

Sungjong Seo <sj1557.seo@samsung.com> writes:

> To clarify MAJOR/MINOR number of a mounted device, fat_msg prints prefix
> that includes them.

Hm, why do we need the major/minor (why can't use sysfs to resolve if
need), and why do you care only fat?

Thanks.

> Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
> ---
>  fs/fat/fat.h  | 2 +-
>  fs/fat/misc.c | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> index 66cf4778cf3b..538bcb3e28e1 100644
> --- a/fs/fat/fat.h
> +++ b/fs/fat/fat.h
> @@ -435,7 +435,7 @@ void __fat_fs_error(struct super_block *sb, int report, const char *fmt, ...);
>  #define fat_fs_error_ratelimit(sb, fmt, args...) \
>  	__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ## args)
>  
> -#define FAT_PRINTK_PREFIX "%sFAT-fs (%s): "
> +#define FAT_PRINTK_PREFIX "%sFAT-fs (%s[%d:%d]): "
>  #define fat_msg(sb, level, fmt, args...)				\
>  do {									\
>  	printk_index_subsys_emit(FAT_PRINTK_PREFIX, level, fmt, ##args);\
> diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> index c7a2d27120ba..6672cefc5484 100644
> --- a/fs/fat/misc.c
> +++ b/fs/fat/misc.c
> @@ -9,6 +9,7 @@
>  
>  #include "fat.h"
>  #include <linux/iversion.h>
> +#include <linux/blkdev.h>
>  
>  /*
>   * fat_fs_error reports a file system problem that might indicate fa data
> @@ -59,7 +60,8 @@ void _fat_msg(struct super_block *sb, const char *level, const char *fmt, ...)
>  	va_start(args, fmt);
>  	vaf.fmt = fmt;
>  	vaf.va = &args;
> -	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id, &vaf);
> +	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id,
> +		MAJOR(sb->s_dev), MINOR(sb->s_dev), &vaf);
>  	va_end(args);
>  }

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

RE: [PATCH] fat: print s_dev via fat_msg

Posted by Sungjong Seo 1 year, 5 months ago

> Sungjong Seo <sj1557.seo@samsung.com> writes:
> 
> > To clarify MAJOR/MINOR number of a mounted device, fat_msg prints prefix
> > that includes them.
> 
> Hm, why do we need the major/minor (why can't use sysfs to resolve if
> need), and why do you care only fat?
> Thanks.
You're right, if you can access to sysfs on a system, this might not
be useful. However, when analyzing problems based on logs, s_dev can be
very helpful for identifying devices. This is because, in systems like
Android, a filesystem gets mounted on a device node with a nickname
like public:179,1.

I think it would be really useful if applied to representative filesystems
for removable storage devices such as fat and exfat. So I will send the
similar PR to exfat as well.

Thanks.
> 
> > Signed-off-by: Sungjong Seo <sj1557.seo@samsung.com>
> > ---
> >  fs/fat/fat.h  | 2 +-
> >  fs/fat/misc.c | 4 +++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/fat/fat.h b/fs/fat/fat.h
> > index 66cf4778cf3b..538bcb3e28e1 100644
> > --- a/fs/fat/fat.h
> > +++ b/fs/fat/fat.h
> > @@ -435,7 +435,7 @@ void __fat_fs_error(struct super_block *sb, int
> report, const char *fmt, ...);
> >  #define fat_fs_error_ratelimit(sb, fmt, args...) \
> >  	__fat_fs_error(sb, __ratelimit(&MSDOS_SB(sb)->ratelimit), fmt , ##
> args)
> >
> > -#define FAT_PRINTK_PREFIX "%sFAT-fs (%s): "
> > +#define FAT_PRINTK_PREFIX "%sFAT-fs (%s[%d:%d]): "
> >  #define fat_msg(sb, level, fmt, args...)				\
> >  do {
\
> >  	printk_index_subsys_emit(FAT_PRINTK_PREFIX, level, fmt, ##args);\
> > diff --git a/fs/fat/misc.c b/fs/fat/misc.c
> > index c7a2d27120ba..6672cefc5484 100644
> > --- a/fs/fat/misc.c
> > +++ b/fs/fat/misc.c
> > @@ -9,6 +9,7 @@
> >
> >  #include "fat.h"
> >  #include <linux/iversion.h>
> > +#include <linux/blkdev.h>
> >
> >  /*
> >   * fat_fs_error reports a file system problem that might indicate fa
> data
> > @@ -59,7 +60,8 @@ void _fat_msg(struct super_block *sb, const char
> *level, const char *fmt, ...)
> >  	va_start(args, fmt);
> >  	vaf.fmt = fmt;
> >  	vaf.va = &args;
> > -	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id, &vaf);
> > +	_printk(FAT_PRINTK_PREFIX "%pV\n", level, sb->s_id,
> > +		MAJOR(sb->s_dev), MINOR(sb->s_dev), &vaf);
> >  	va_end(args);
> >  }
> 
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: print s_dev via fat_msg

Posted by OGAWA Hirofumi 1 year, 5 months ago

"Sungjong Seo" <sj1557.seo@samsung.com> writes:

>> Sungjong Seo <sj1557.seo@samsung.com> writes:
>> 
>> > To clarify MAJOR/MINOR number of a mounted device, fat_msg prints prefix
>> > that includes them.
>> 
>> Hm, why do we need the major/minor (why can't use sysfs to resolve if
>> need), and why do you care only fat?
>> Thanks.
> You're right, if you can access to sysfs on a system, this might not
> be useful. However, when analyzing problems based on logs, s_dev can be
> very helpful for identifying devices. This is because, in systems like
> Android, a filesystem gets mounted on a device node with a nickname
> like public:179,1.
>
> I think it would be really useful if applied to representative filesystems
> for removable storage devices such as fat and exfat. So I will send the
> similar PR to exfat as well.

So this is for the naming policy like android? And why don't you care
the other places (like vfs) that using ->s_id?

Because I dislike to use the inconsitent stuff, some logs are "sda3" and
some logs are "sda3[8:3]".

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

RE: [PATCH] fat: print s_dev via fat_msg

Posted by Sungjong Seo 1 year, 5 months ago

> "Sungjong Seo" <sj1557.seo@samsung.com> writes:
> 
> >> Sungjong Seo <sj1557.seo@samsung.com> writes:
> >>
> >> > To clarify MAJOR/MINOR number of a mounted device, fat_msg prints
> prefix
> >> > that includes them.
> >>
> >> Hm, why do we need the major/minor (why can't use sysfs to resolve if
> >> need), and why do you care only fat?
> >> Thanks.
> > You're right, if you can access to sysfs on a system, this might not
> > be useful. However, when analyzing problems based on logs, s_dev can be
> > very helpful for identifying devices. This is because, in systems like
> > Android, a filesystem gets mounted on a device node with a nickname
> > like public:179,1.
> >
> > I think it would be really useful if applied to representative
> filesystems
> > for removable storage devices such as fat and exfat. So I will send the
> > similar PR to exfat as well.
> 
> So this is for the naming policy like android?
Yes, but I think it is just one of examples.
>
> And why don't you care the other places (like vfs) that using ->s_id?
Because, I think it's enough to change fat-fs and exfat-fs.
> 
> Because I dislike to use the inconsitent stuff, some logs are "sda3" and
> some logs are "sda3[8:3]".
Do you mean consistency between all logs under Linux VFS?
If so, I think it's meaningless. As mentioned above, this patch
helps analyze removable storage devices, so it would be nice if it could
be applied to both fat-fs and exfat-fs.

Anyway, this is just my opinion and I think you might have a different one.
Thanks!
> 
> Thanks.
> --
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>