2009-03-31 02:41:28

by Manish Katiyar

[permalink] [raw]
Subject: Re: [PATCH] : Add dev-id and inode number for debugging in init_special_inode in fs/inode.c

On Tue, Mar 31, 2009 at 5:03 AM, Andrew Morton
<[email protected]> wrote:
> On Mon, 23 Mar 2009 08:46:12 +0530
> Manish Katiyar <[email protected]> wrote:
>
>> Hi Andrew,
>>
>> Below patch adds device-id and inode number for better debugging.
>>
>> Signed-off-by: Manish Katiyar <[email protected]>
>> ---
>> ?fs/inode.c | ? ?5 +++--
>> ?1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 913ab2d..f75e9a4 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -1540,7 +1540,8 @@ void init_special_inode(struct inode *inode,
>> umode_t mode, dev_t rdev)
>> ? ? ? else if (S_ISSOCK(mode))
>> ? ? ? ? ? ? ? inode->i_fop = &bad_sock_fops;
>> ? ? ? else
>> - ? ? ? ? ? ? printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o)\n",
>> - ? ? ? ? ? ? ? ? ? ?mode);
>> + ? ? ? ? ? ? printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o) for"
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? " inode %s:%lu\n", mode, inode->i_sb->s_id,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? inode->i_ino);
>> ?}
>> ?EXPORT_SYMBOL(init_special_inode);
>
> I don't think this is really a trivial patch. ?Please cc linux-kernel
> unless the patch is really really trivail.
>
> The changelog falied to give any reason for making this change. ?It
> should have done so! ?I cannot work out the rationale from looking at
> the code change.

Hi Andrew,
[resending it with updated changelog and ccing linux-kernel]

Below patch adds device-id and inode number for better debugging. This
was suggested by Andreas in one of the threads
http://article.gmane.org/gmane.comp.file-systems.ext4/12062 .

Signed-off-by: Manish Katiyar <[email protected]>
---
fs/inode.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 913ab2d..f75e9a4 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1540,7 +1540,8 @@ void init_special_inode(struct inode *inode,
umode_t mode, dev_t rdev)
else if (S_ISSOCK(mode))
inode->i_fop = &bad_sock_fops;
else
- printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o)\n",
- mode);
+ printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o) for"
+ " inode %s:%lu\n", mode, inode->i_sb->s_id,
+ inode->i_ino);
}
EXPORT_SYMBOL(init_special_inode);
--
1.5.4.3



PS: I have also attached the patch, in case it gets word wrapped.

Thanks -
Manish

>
>
>
>


Attachments:
0001-Add-dev-id-and-inode-number-for-debugging-in-init_sp.patch (898.00 B)

2009-04-01 21:54:51

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] : Add dev-id and inode number for debugging in init_special_inode in fs/inode.c

On Tue, 31 Mar 2009 08:11:00 +0530
Manish Katiyar <[email protected]> wrote:

> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1540,7 +1540,8 @@ void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
> else if (S_ISSOCK(mode))
> inode->i_fop = &bad_sock_fops;
> else
> - printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o)\n",
> - mode);
> + printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o) for"
> + " inode %s:%lu\n", mode, inode->i_sb->s_id,
> + inode->i_ino);
> }

Is ther a guarantee that inode->i_sb and inode->i_ino have been initialised
at this time?

2009-04-03 10:35:54

by Manish Katiyar

[permalink] [raw]
Subject: Re: [PATCH] : Add dev-id and inode number for debugging in init_special_inode in fs/inode.c

On Thu, Apr 2, 2009 at 3:20 AM, Andrew Morton <[email protected]> wrote:
>
> On Tue, 31 Mar 2009 08:11:00 +0530
> Manish Katiyar <[email protected]> wrote:
>
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -1540,7 +1540,8 @@ void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
> > ? ? ? else if (S_ISSOCK(mode))
> > ? ? ? ? ? ? ? inode->i_fop = &bad_sock_fops;
> > ? ? ? else
> > - ? ? ? ? ? ? printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o)\n",
> > - ? ? ? ? ? ? ? ? ? ?mode);
> > + ? ? ? ? ? ? printk(KERN_DEBUG "init_special_inode: bogus i_mode (%o) for"
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? " inode %s:%lu\n", mode, inode->i_sb->s_id,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? inode->i_ino);
> > ?}
>
> Is ther a guarantee that inode->i_sb and inode->i_ino have been initialised
> at this time?

Hi Andrew,

I checked the code and most of the places we are trying to access
inode->i_sb , inode->i_ino before this function call or after this,
without any checks. So looks like we will have the values at this
point of time.

Thanks -
Manish