From: Namjae Jeon <[email protected]>
The root directory inode is added to the directory hash table in
fat_attach when mounted with nfs option. Since root dentry never
expires until umount, fat_dget never fails to return the root inode
for subdirectories of root inode.i.e., parent_logstart cannot be zero.
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 | 51 ++++++++++++++++++++++-----------------------------
1 file changed, 22 insertions(+), 29 deletions(-)
diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
index a4e6ac7..671a75d 100644
--- a/fs/fat/nfs.c
+++ b/fs/fat/nfs.c
@@ -274,36 +274,29 @@ struct dentry *fat_get_parent(struct dentry *child_dir)
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);
+ 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);
}
out:
brelse(dotdot_bh);
--
1.7.9.5
Namjae Jeon <[email protected]> writes:
> From: Namjae Jeon <[email protected]>
>
> The root directory inode is added to the directory hash table in
> fat_attach when mounted with nfs option. Since root dentry never
> expires until umount, fat_dget never fails to return the root inode
> for subdirectories of root inode.i.e., parent_logstart cannot be zero.
Sorry, I will review at this weekend.
Thanks.
> 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 | 51 ++++++++++++++++++++++-----------------------------
> 1 file changed, 22 insertions(+), 29 deletions(-)
>
> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
> index a4e6ac7..671a75d 100644
> --- a/fs/fat/nfs.c
> +++ b/fs/fat/nfs.c
> @@ -274,36 +274,29 @@ struct dentry *fat_get_parent(struct dentry *child_dir)
> 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);
> + 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);
> }
> out:
> brelse(dotdot_bh);
--
OGAWA Hirofumi <[email protected]>
2012/10/31, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> From: Namjae Jeon <[email protected]>
>>
>> The root directory inode is added to the directory hash table in
>> fat_attach when mounted with nfs option. Since root dentry never
>> expires until umount, fat_dget never fails to return the root inode
>> for subdirectories of root inode.i.e., parent_logstart cannot be zero.
>
> Sorry, I will review at this weekend.
No problem, Thank you.
>
> Thanks.
>
>> 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 | 51 ++++++++++++++++++++++-----------------------------
>> 1 file changed, 22 insertions(+), 29 deletions(-)
>>
>> diff --git a/fs/fat/nfs.c b/fs/fat/nfs.c
>> index a4e6ac7..671a75d 100644
>> --- a/fs/fat/nfs.c
>> +++ b/fs/fat/nfs.c
>> @@ -274,36 +274,29 @@ struct dentry *fat_get_parent(struct dentry
>> *child_dir)
>> 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);
>> + 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);
>> }
>> out:
>> brelse(dotdot_bh);
>
> --
> OGAWA Hirofumi <[email protected]>
>
Namjae Jeon <[email protected]> writes:
> + 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);
> }
> out:
> brelse(dotdot_bh);
I started to understand what is doing this with cleanup. It is really
strange and duplicates many codes with bugs.
So, please replace whole FAT nfs patches in -mm to review whole at next
patchset. And sorry, but please think whole more carefully.
Thanks.
--
OGAWA Hirofumi <[email protected]>
2012/11/5, OGAWA Hirofumi <[email protected]>:
> Namjae Jeon <[email protected]> writes:
>
>> + 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);
>> }
>> out:
>> brelse(dotdot_bh);
>
> I started to understand what is doing this with cleanup. It is really
> strange and duplicates many codes with bugs.
>
> So, please replace whole FAT nfs patches in -mm to review whole at next
> patchset. And sorry, but please think whole more carefully.
Hi OGAWA.
Okay, I will rework all of patches as your comment.
Thanks for review.
>
> Thanks.
> --
> OGAWA Hirofumi <[email protected]>
>