From: Namjae Jeon <[email protected]>
fat_nfs_get_inode does not hold i_mutex of parent directory.So add
lock to fat_build_inode.
Signed-off-by: Namjae Jeon <[email protected]>
Signed-off-by: Ravishankar N <[email protected]>
Signed-off-by: Amit Sahrawat <[email protected]>
---
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
On Sun, 28 Oct 2012 10:53:43 +0900
Namjae Jeon <[email protected]> wrote:
> From: Namjae Jeon <[email protected]>
>
> 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?
2012/10/30, Andrew Morton <[email protected]>:
> On Sun, 28 Oct 2012 10:53:43 +0900
> Namjae Jeon <[email protected]> wrote:
>
>> From: Namjae Jeon <[email protected]>
>>
>> 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!
>
Namjae Jeon <[email protected]> writes:
> 2012/10/30, Andrew Morton <[email protected]>:
>> On Sun, 28 Oct 2012 10:53:43 +0900
>> Namjae Jeon <[email protected]> wrote:
>>
>>> From: Namjae Jeon <[email protected]>
>>>
>>> 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 <[email protected]>