* [PATCH 5/5] fat: add mutex lock to fat_build_inode
@ 2012-10-28 1:53 Namjae Jeon
2012-10-29 23:49 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Namjae Jeon @ 2012-10-28 1:53 UTC (permalink / raw)
To: hirofumi, akpm
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Namjae Jeon,
Ravishankar N, Amit Sahrawat
From: Namjae Jeon <namjae.jeon@samsung.com>
fat_nfs_get_inode does not hold i_mutex of parent directory.So add
lock to fat_build_inode.
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Ravishankar N <ravi.n1@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
2012-10-28 1:53 [PATCH 5/5] fat: add mutex lock to fat_build_inode Namjae Jeon
@ 2012-10-29 23:49 ` Andrew Morton
2012-10-30 4:24 ` Namjae Jeon
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2012-10-29 23:49 UTC (permalink / raw)
To: Namjae Jeon
Cc: hirofumi, linux-fsdevel, linux-kernel, Namjae Jeon, Ravishankar N,
Amit Sahrawat
On Sun, 28 Oct 2012 10:53:43 +0900
Namjae Jeon <linkinjeon@gmail.com> wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>
> 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?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
2012-10-29 23:49 ` Andrew Morton
@ 2012-10-30 4:24 ` Namjae Jeon
2012-10-31 0:46 ` OGAWA Hirofumi
0 siblings, 1 reply; 4+ messages in thread
From: Namjae Jeon @ 2012-10-30 4:24 UTC (permalink / raw)
To: Andrew Morton
Cc: hirofumi, linux-fsdevel, linux-kernel, Namjae Jeon, Ravishankar N,
Amit Sahrawat
2012/10/30, Andrew Morton <akpm@linux-foundation.org>:
> On Sun, 28 Oct 2012 10:53:43 +0900
> Namjae Jeon <linkinjeon@gmail.com> wrote:
>
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> 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!
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] fat: add mutex lock to fat_build_inode
2012-10-30 4:24 ` Namjae Jeon
@ 2012-10-31 0:46 ` OGAWA Hirofumi
0 siblings, 0 replies; 4+ messages in thread
From: OGAWA Hirofumi @ 2012-10-31 0:46 UTC (permalink / raw)
To: Namjae Jeon
Cc: Andrew Morton, linux-fsdevel, linux-kernel, Namjae Jeon,
Ravishankar N, Amit Sahrawat
Namjae Jeon <linkinjeon@gmail.com> writes:
> 2012/10/30, Andrew Morton <akpm@linux-foundation.org>:
>> On Sun, 28 Oct 2012 10:53:43 +0900
>> Namjae Jeon <linkinjeon@gmail.com> wrote:
>>
>>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>>
>>> 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 <hirofumi@mail.parknet.co.jp>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-31 0:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-28 1:53 [PATCH 5/5] fat: add mutex lock to fat_build_inode Namjae Jeon
2012-10-29 23:49 ` Andrew Morton
2012-10-30 4:24 ` Namjae Jeon
2012-10-31 0:46 ` OGAWA Hirofumi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).