2012-10-07 08:32:58

by Namjae Jeon

[permalink] [raw]
Subject: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

From: Namjae Jeon <[email protected]>

This patch enables rebuilding of directory inodes which are not present
in the cache.This is done by traversing the disk clusters to find the
directory entry of the parent directory and using its i_pos to build the
inode.

Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Ravishankar N <[email protected]>
Signed-off-by: Amit Sahrawat <[email protected]>
---
fs/fat/nfs.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 128 insertions(+), 4 deletions(-)

diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
index 156903b..c1cb12b 100644
--- a/fs/fat/nfs.c
+++ b/fs/fat/nfs.c
@@ -166,6 +166,93 @@ struct dentry *fat_fh_to_parent(struct super_block *sb, struct fid *fh,
}

/*
+ * Read the directory entries of 'search_clus' and find the entry
+ * which contains 'match_ipos' for the starting cluster.If the entry
+ * is found, rebuild its inode.
+ */
+static struct inode *fat_traverse_cluster(struct super_block *sb,
+ int search_clus, int match_ipos)
+{
+ struct msdos_sb_info *sbi = MSDOS_SB(sb);
+ struct buffer_head *bh;
+ sector_t blknr;
+ int parent_ipos, search_ipos;
+ int i;
+ struct msdos_dir_entry *de;
+ struct inode *inode = NULL;
+ int iterations = sbi->cluster_size >> sb->s_blocksize_bits;
+ blknr = fat_clus_to_blknr(sbi, search_clus);
+
+ do {
+ bh = sb_bread(sb, blknr);
+ if (!bh) {
+ fat_msg(sb, KERN_ERR,
+ "NFS:unable to read block(%llu) while traversing cluster(%d)",
+ (llu)blknr, search_clus);
+ inode = ERR_PTR(-EIO);
+ goto out;
+ }
+ de = (struct msdos_dir_entry *)bh->b_data;
+ for (i = 0; i < sbi->dir_per_block; i++) {
+ if (de[i].name[0] == FAT_ENT_FREE) {
+ /*Reached end of directory*/
+ brelse(bh);
+ inode = ERR_PTR(-ENODATA);
+ goto out;
+ }
+ if (de[i].name[0] == DELETED_FLAG)
+ continue;
+ if (de[i].attr == ATTR_EXT)
+ continue;
+ if (!(de[i].attr & ATTR_DIR))
+ continue;
+ else {
+ search_ipos = fat_get_start(sbi, &de[i]);
+ if (search_ipos == match_ipos) {
+ /*Success.Now build the inode*/
+ parent_ipos = (loff_t)i +
+ (blknr << sbi->dir_per_block_bits);
+ inode = fat_build_inode(sb, &de[i],
+ parent_ipos);
+ brelse(bh);
+ goto out;
+ }
+ }
+ }
+ brelse(bh);
+ blknr += 1;
+ } while (--iterations > 0);
+out:
+ return inode;
+}
+
+/*
+ * Read the FAT to find the next cluster in the chain
+ * corresponding to 'search_clus'.
+ */
+static int fat_read_next_clus(struct super_block *sb, int search_clus)
+{
+ struct msdos_sb_info *sbi = MSDOS_SB(sb);
+ /*bits 31 to 7 give relative sector number*/
+ sector_t blknr = search_clus >> 7;
+ /*bits 6 to 0 give offset*/
+ unsigned int offset = search_clus & 0x7F;
+ __le32 *address;
+ unsigned int next_cluster;
+ struct buffer_head *bh = sb_bread(sb, blknr + sbi->fat_start);
+ if (!bh) {
+ fat_msg(sb, KERN_ERR,
+ "NFS:unable to read block(%llu) for finding the next cluster in FAT chain",
+ (llu)blknr);
+ return -EIO;
+ }
+ address = (__le32 *) bh->b_data;
+ next_cluster = (le32_to_cpu(address[offset])) & 0x0FFFFFFF;
+ brelse(bh);
+ return next_cluster;
+}
+
+/*
* Find the parent for a directory that is not currently connected to
* the filesystem root.
*
@@ -174,15 +261,52 @@ struct dentry *fat_fh_to_parent(struct super_block *sb, struct fid *fh,
struct dentry *fat_get_parent(struct dentry *child_dir)
{
struct super_block *sb = child_dir->d_sb;
- struct buffer_head *bh = NULL;
+ struct buffer_head *dotdot_bh = NULL, *parent_bh = NULL;
struct msdos_dir_entry *de;
struct inode *parent_inode = NULL;
+ struct msdos_sb_info *sbi = MSDOS_SB(sb);
+ int parent_logstart;
+ int search_clus, clus_to_match;
+ sector_t blknr;

- if (!fat_get_dotdot_entry(child_dir->d_inode, &bh, &de)) {
- int parent_logstart = fat_get_start(MSDOS_SB(sb), de);
+ if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
+ parent_logstart = fat_get_start(sbi, de);
parent_inode = fat_dget(sb, parent_logstart);
+ if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
+ goto out;
+ if (!parent_logstart)
+ /*logstart of dotdot entry is zero if
+ * if the directory's parent is root
+ */
+ parent_inode = sb->s_root->d_inode;
+ else {
+ blknr = fat_clus_to_blknr(sbi, parent_logstart);
+ parent_bh = sb_bread(sb, blknr);
+ if (!parent_bh) {
+ fat_msg(sb, KERN_ERR,
+ "NFS:unable to read cluster of parent directory");
+ goto out;
+ }
+ de = (struct msdos_dir_entry *) parent_bh->b_data;
+ clus_to_match = fat_get_start(sbi, &de[0]);
+ search_clus = fat_get_start(sbi, &de[1]);
+ if (!search_clus)
+ search_clus = sbi->root_cluster;
+ brelse(parent_bh);
+ do {
+ parent_inode = fat_traverse_cluster(sb,
+ search_clus, clus_to_match);
+ if (IS_ERR(parent_inode) || parent_inode)
+ break;
+ search_clus = fat_read_next_clus(sb,
+ search_clus);
+ if (search_clus < 0)
+ break;
+ } while (search_clus != FAT_ENT_EOF);
+ }
}
- brelse(bh);
+out:
+ brelse(dotdot_bh);

return d_obtain_alias(parent_inode);
}
--
1.7.9.5


2012-10-13 09:27:58

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

Namjae Jeon <[email protected]> writes:

> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
> + parent_logstart = fat_get_start(sbi, de);
> parent_inode = fat_dget(sb, parent_logstart);
> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
> + goto out;
> + if (!parent_logstart)
> + /*logstart of dotdot entry is zero if
> + * if the directory's parent is root
> + */
> + parent_inode = sb->s_root->d_inode;

get_parent() should not be called for root dir, right?

> + blknr = fat_clus_to_blknr(sbi, parent_logstart);
> + parent_bh = sb_bread(sb, blknr);
> + if (!parent_bh) {
> + fat_msg(sb, KERN_ERR,
> + "NFS:unable to read cluster of parent directory");
> + goto out;
> + }
> + de = (struct msdos_dir_entry *) parent_bh->b_data;
> + clus_to_match = fat_get_start(sbi, &de[0]);
> + search_clus = fat_get_start(sbi, &de[1]);
> + if (!search_clus)
> + search_clus = sbi->root_cluster;
> + brelse(parent_bh);
> + do {
> + parent_inode = fat_traverse_cluster(sb,
> + search_clus, clus_to_match);
> + if (IS_ERR(parent_inode) || parent_inode)
> + break;
> + search_clus = fat_read_next_clus(sb,
> + search_clus);
> + if (search_clus < 0)
> + break;
> + } while (search_clus != FAT_ENT_EOF);
> + }

Please make this part as own function at least. And this is doing same
thing with readdir, so we will have to clean this up as I said before.
--
OGAWA Hirofumi <[email protected]>

2012-10-13 14:03:35

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/13 OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
>> + parent_logstart = fat_get_start(sbi, de);
>> parent_inode = fat_dget(sb, parent_logstart);
>> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
>> + goto out;
>> + if (!parent_logstart)
>> + /*logstart of dotdot entry is zero if
>> + * if the directory's parent is root
>> + */
>> + parent_inode = sb->s_root->d_inode;
>
> get_parent() should not be called for root dir, right?
Yes.
>
>> + blknr = fat_clus_to_blknr(sbi, parent_logstart);
>> + parent_bh = sb_bread(sb, blknr);
>> + if (!parent_bh) {
>> + fat_msg(sb, KERN_ERR,
>> + "NFS:unable to read cluster of parent directory");
>> + goto out;
>> + }
>> + de = (struct msdos_dir_entry *) parent_bh->b_data;
>> + clus_to_match = fat_get_start(sbi, &de[0]);
>> + search_clus = fat_get_start(sbi, &de[1]);
>> + if (!search_clus)
>> + search_clus = sbi->root_cluster;
>> + brelse(parent_bh);
>> + do {
>> + parent_inode = fat_traverse_cluster(sb,
>> + search_clus, clus_to_match);
>> + if (IS_ERR(parent_inode) || parent_inode)
>> + break;
>> + search_clus = fat_read_next_clus(sb,
>> + search_clus);
>> + if (search_clus < 0)
>> + break;
>> + } while (search_clus != FAT_ENT_EOF);
>> + }
>
> Please make this part as own function at least. And this is doing same
> thing with readdir, so we will have to clean this up as I said before.
Okay, I will.

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

2012-10-13 14:35:53

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

Namjae Jeon <[email protected]> writes:

> 2012/10/13 OGAWA Hirofumi <[email protected]>:
>> Namjae Jeon <[email protected]> writes:
>>
>>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
>>> + parent_logstart = fat_get_start(sbi, de);
>>> parent_inode = fat_dget(sb, parent_logstart);
>>> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
>>> + goto out;
>>> + if (!parent_logstart)
>>> + /*logstart of dotdot entry is zero if
>>> + * if the directory's parent is root
>>> + */
>>> + parent_inode = sb->s_root->d_inode;
>>
>> get_parent() should not be called for root dir, right?
> Yes.

Ah, actual question was - subdir of root can be passed to this? I guess
it is possible...
--
OGAWA Hirofumi <[email protected]>

2012-10-13 15:14:46

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/13 OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> 2012/10/13 OGAWA Hirofumi <[email protected]>:
>>> Namjae Jeon <[email protected]> writes:
>>>
>>>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
>>>> + parent_logstart = fat_get_start(sbi, de);
>>>> parent_inode = fat_dget(sb, parent_logstart);
>>>> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
>>>> + goto out;
>>>> + if (!parent_logstart)
>>>> + /*logstart of dotdot entry is zero if
>>>> + * if the directory's parent is root
>>>> + */
>>>> + parent_inode = sb->s_root->d_inode;
>>>
>>> get_parent() should not be called for root dir, right?
>> Yes.
>
> Ah, actual question was - subdir of root can be passed to this? I guess
> it is possible...
Yes, you're right.

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

2012-10-13 16:50:37

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

Namjae Jeon <[email protected]> writes:

>>>>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
>>>>> + parent_logstart = fat_get_start(sbi, de);
>>>>> parent_inode = fat_dget(sb, parent_logstart);
>>>>> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
>>>>> + goto out;
>>>>> + if (!parent_logstart)
>>>>> + /*logstart of dotdot entry is zero if
>>>>> + * if the directory's parent is root
>>>>> + */
>>>>> + parent_inode = sb->s_root->d_inode;
>>>>
>>>> get_parent() should not be called for root dir, right?
>>> Yes.
>>
>> Ah, actual question was - subdir of root can be passed to this? I guess
>> it is possible...
> Yes, you're right.

OK, I think I got where is wrong. If it is the subdir of rootdir,
fat_dget() should get the inode of root?

So, I guess if parent_logstart == 0, it is BUG().
--
OGAWA Hirofumi <[email protected]>

2012-10-15 07:28:32

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/14, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>>>>>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de))
>>>>>> {
>>>>>> + parent_logstart = fat_get_start(sbi, de);
>>>>>> parent_inode = fat_dget(sb, parent_logstart);
>>>>>> + if (parent_inode || sbi->options.nfs !=
>>>>>> FAT_NFS_NOSTALE_RO)
>>>>>> + goto out;
>>>>>> + if (!parent_logstart)
>>>>>> + /*logstart of dotdot entry is zero if
>>>>>> + * if the directory's parent is root
>>>>>> + */
>>>>>> + parent_inode = sb->s_root->d_inode;
>>>>>
>>>>> get_parent() should not be called for root dir, right?
>>>> Yes.
>>>
>>> Ah, actual question was - subdir of root can be passed to this? I guess
>>> it is possible...
>> Yes, you're right.
>
> OK, I think I got where is wrong. If it is the subdir of rootdir,
> fat_dget() should get the inode of root?
That is correct. Whenever fat_dget() is called for sub-directories
which is in root folder, we assign root inode as parent inode.
>
> So, I guess if parent_logstart == 0, it is BUG().
Would you explain more why you think it is BUG() ?

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

2012-10-15 07:57:20

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

Namjae Jeon <[email protected]> writes:

>> OK, I think I got where is wrong. If it is the subdir of rootdir,
>> fat_dget() should get the inode of root?
> That is correct. Whenever fat_dget() is called for sub-directories
> which is in root folder, we assign root inode as parent inode.
>>
>> So, I guess if parent_logstart == 0, it is BUG().
> Would you explain more why you think it is BUG() ?

Because root dentry is never expired until umount. So, fat_dget()
shouldn't never fail to get inode for subdir of rootdir. Otherwise,
"stale_rw" will not be working in the case even if there is cache.

I.e. I think we don't need to check parent_logstart == 0.

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

2012-10-15 13:20:18

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/15 OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>>> OK, I think I got where is wrong. If it is the subdir of rootdir,
>>> fat_dget() should get the inode of root?
>> That is correct. Whenever fat_dget() is called for sub-directories
>> which is in root folder, we assign root inode as parent inode.
>>>
>>> So, I guess if parent_logstart == 0, it is BUG().
>> Would you explain more why you think it is BUG() ?
>
> Because root dentry is never expired until umount. So, fat_dget()
> shouldn't never fail to get inode for subdir of rootdir. Otherwise,
> "stale_rw" will not be working in the case even if there is cache.
>
> I.e. I think we don't need to check parent_logstart == 0.
Yes, You're right. I found this condition is really not needed when I
check again.
I will remove this code.
Thanks OGAWA!
>
> Thanks.
> --
> OGAWA Hirofumi <[email protected]>

2012-10-23 07:34:42

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/13, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> + if (!fat_get_dotdot_entry(child_dir->d_inode, &dotdot_bh, &de)) {
>> + parent_logstart = fat_get_start(sbi, de);
>> parent_inode = fat_dget(sb, parent_logstart);
>> + if (parent_inode || sbi->options.nfs != FAT_NFS_NOSTALE_RO)
>> + goto out;
>> + if (!parent_logstart)
>> + /*logstart of dotdot entry is zero if
>> + * if the directory's parent is root
>> + */
>> + parent_inode = sb->s_root->d_inode;
>
> get_parent() should not be called for root dir, right?
>
>> + blknr = fat_clus_to_blknr(sbi, parent_logstart);
>> + parent_bh = sb_bread(sb, blknr);
>> + if (!parent_bh) {
>> + fat_msg(sb, KERN_ERR,
>> + "NFS:unable to read cluster of parent directory");
>> + goto out;
>> + }
>> + de = (struct msdos_dir_entry *) parent_bh->b_data;
>> + clus_to_match = fat_get_start(sbi, &de[0]);
>> + search_clus = fat_get_start(sbi, &de[1]);
>> + if (!search_clus)
>> + search_clus = sbi->root_cluster;
>> + brelse(parent_bh);
>> + do {
>> + parent_inode = fat_traverse_cluster(sb,
>> + search_clus, clus_to_match);
>> + if (IS_ERR(parent_inode) || parent_inode)
>> + break;
>> + search_clus = fat_read_next_clus(sb,
>> + search_clus);
>> + if (search_clus < 0)
>> + break;
>> + } while (search_clus != FAT_ENT_EOF);
>> + }
>
Hi. OGAWA.
I have a question.
> Please make this part as own function at least.
Okay, I will make own function.
>And this is doing same
> thing with readdir, so we will have to clean this up as I said before.
When I checked, I didn't understand about same thing readdir and this
function yet. Because even though minor conditions match but
functionality wise both are different.

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

2012-10-23 07:53:12

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

Namjae Jeon <[email protected]> writes:

>>And this is doing same
>> thing with readdir, so we will have to clean this up as I said before.
> When I checked, I didn't understand about same thing readdir and this
> function yet. Because even though minor conditions match but
> functionality wise both are different.

Hm, I may not be understanding it correctly. I thought the both are
looking the key up? The key of readdir is the name, the key of this is
the logstart.

So, I thought it should be similar. However, I just may be wrong
somewhere... Well, even if those are possible to merge, it should be
separated patch though.

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

2012-10-23 11:38:05

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH v4 3/4] fat (exportfs): rebuild directory-inode if fat_dget() fails

2012/10/23, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>>>And this is doing same
>>> thing with readdir, so we will have to clean this up as I said before.
>> When I checked, I didn't understand about same thing readdir and this
>> function yet. Because even though minor conditions match but
>> functionality wise both are different.
>
> Hm, I may not be understanding it correctly. I thought the both are
> looking the key up? The key of readdir is the name, the key of this is
> the logstart.
>
> So, I thought it should be similar. However, I just may be wrong
> somewhere... Well, even if those are possible to merge, it should be
> separated patch though.
Okay, I see. I will post changes after verifying.

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