Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
Namjae Jeon writes: > 2012/10/30, Andrew Morton : >> On Sun, 28 Oct 2012 10:53:43 +0900 >> Namjae Jeon wrote: >> >>> From: Namjae Jeon >>> >>> fat_nfs_get_inode does not hold i_mutex of parent directory.So add >>> lock to fat_build_inode. >> > Hi. Andrew. >> Well.. why? Presumably this patch fixes some race. A good >> description of that race would be useful - partly because others may >> then be able to suggest alternative ways of fixing that bug. > We are making use of fat_build_inode to build the inode using 'i_pos'. > Since, this function is local to FAT and when mounted over NFS. We can > make use of FAT parallely from local NFS Server and mounted from NFS > client. So, in order to avoid race to multiple regeneration for the > same 'i_pos' - we have introduced this locking. This lock fixes the NFS patches. FAT inode is embedded into directory. So usual local ->lookup path is exclusive by inode->i_mutex. But NFS patches (current -mm, IIRC) introduce the new path for FH => inode lookup. So, this lock is introduced. -- OGAWA Hirofumi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
2012/10/30, Andrew Morton : > On Sun, 28 Oct 2012 10:53:43 +0900 > Namjae Jeon wrote: > >> From: Namjae Jeon >> >> fat_nfs_get_inode does not hold i_mutex of parent directory.So add >> lock to fat_build_inode. > Hi. Andrew. > Well.. why? Presumably this patch fixes some race. A good > description of that race would be useful - partly because others may > then be able to suggest alternative ways of fixing that bug. We are making use of fat_build_inode to build the inode using 'i_pos'. Since, this function is local to FAT and when mounted over NFS. We can make use of FAT parallely from local NFS Server and mounted from NFS client. So, in order to avoid race to multiple regeneration for the same 'i_pos' - we have introduced this locking. > > I'll merge these patches for some testing. > > I did merge these patches three weekes ago: > > fat-modify-nfs-mount-option.patch > fat-exportfs-rebuild-inode-if-ilookup-fails.patch > fat-exportfs-rebuild-inode-if-ilookup-fails-fix.patch > fat-exportfs-rebuild-directory-inode-if-fat_dget-fails.patch > documentation-update-nfs-option-in-filesystem-vfattxt.patch > > But I have no record of Bruce or Ogawa having reviewed/acked them? While the patches were sent for review on LKML and also in the meantime since the functionality was finalised, the patches were picked for merging in the linux tree. But there few comments which were later provided by OGAWA. So, these patches are extending the earlier patch-set by providing some fix-ups and cleanup routines. Thanks! > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
On Sun, 28 Oct 2012 10:53:43 +0900 Namjae Jeon wrote: > From: Namjae Jeon > > fat_nfs_get_inode does not hold i_mutex of parent directory.So add > lock to fat_build_inode. Well.. why? Presumably this patch fixes some race. A good description of that race would be useful - partly because others may then be able to suggest alternative ways of fixing that bug. I'll merge these patches for some testing. I did merge these patches three weekes ago: fat-modify-nfs-mount-option.patch fat-exportfs-rebuild-inode-if-ilookup-fails.patch fat-exportfs-rebuild-inode-if-ilookup-fails-fix.patch fat-exportfs-rebuild-directory-inode-if-fat_dget-fails.patch documentation-update-nfs-option-in-filesystem-vfattxt.patch But I have no record of Bruce or Ogawa having reviewed/acked them? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/5] fat: add mutex lock to fat_build_inode
From: Namjae Jeon fat_nfs_get_inode does not hold i_mutex of parent directory.So add lock to fat_build_inode. Signed-off-by: Namjae Jeon Signed-off-by: Ravishankar N Signed-off-by: Amit Sahrawat --- fs/fat/fat.h |1 + fs/fat/inode.c | 15 +++ 2 files changed, 16 insertions(+) diff --git a/fs/fat/fat.h b/fs/fat/fat.h index 177e94e..811267c 100644 --- a/fs/fat/fat.h +++ b/fs/fat/fat.h @@ -75,6 +75,7 @@ struct msdos_sb_info { unsigned long fsinfo_sector; /* sector number of FAT32 fsinfo */ struct mutex fat_lock; struct mutex s_lock; + struct mutex nfs_build_inode_lock; unsigned int prev_free; /* previously allocated cluster number */ unsigned int free_clusters; /* -1 if undefined */ unsigned int free_clus_valid; /* is free_clusters valid? */ diff --git a/fs/fat/inode.c b/fs/fat/inode.c index 63e0883..a1650ef 100644 --- a/fs/fat/inode.c +++ b/fs/fat/inode.c @@ -443,12 +443,25 @@ static int fat_fill_inode(struct inode *inode, struct msdos_dir_entry *de) return 0; } +static inline void fat_lock_build_inode(struct msdos_sb_info *sbi) +{ + if (sbi->options.nfs == FAT_NFS_NOSTALE_RO) + mutex_lock(&sbi->nfs_build_inode_lock); +} + +static inline void fat_unlock_build_inode(struct msdos_sb_info *sbi) +{ + if (sbi->options.nfs == FAT_NFS_NOSTALE_RO) + mutex_unlock(&sbi->nfs_build_inode_lock); +} + struct inode *fat_build_inode(struct super_block *sb, struct msdos_dir_entry *de, loff_t i_pos) { struct inode *inode; int err; + fat_lock_build_inode(MSDOS_SB(sb)); inode = fat_iget(sb, i_pos); if (inode) goto out; @@ -468,6 +481,7 @@ struct inode *fat_build_inode(struct super_block *sb, fat_attach(inode, i_pos); insert_inode_hash(inode); out: + fat_unlock_build_inode(MSDOS_SB(sb)); return inode; } @@ -1173,6 +1187,7 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, sb->s_magic = MSDOS_SUPER_MAGIC; sb->s_op = &fat_sops; sb->s_export_op = &fat_export_ops; + mutex_init(&sbi->nfs_build_inode_lock); ratelimit_state_init(&sbi->ratelimit, DEFAULT_RATELIMIT_INTERVAL, DEFAULT_RATELIMIT_BURST); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/