On Jun. 15, 2010, 14:19 -0400, Trond Myklebust <trond.myklebust@xxxxxxxxxx> wrote: > On Tue, 2010-06-15 at 13:52 -0400, Fred Isaman wrote: >> On Tue, Jun 15, 2010 at 1:33 PM, Trond Myklebust >> <trond.myklebust@xxxxxxxxxx> wrote: >>> On Tue, 2010-06-15 at 13:32 -0400, Fred Isaman wrote: >>>> On Tue, Jun 15, 2010 at 1:06 PM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: >>>>> On Jun. 14, 2010, 21:46 -0400, Fred Isaman <iisaman@xxxxxxxxxx> wrote: >>>>>> This prepares for the next patch. >>>>>> >>>>>> NOTE this doesn't really fix any current race, since >>>>>> layout going to NULL is OK. But layout changing from NULL to nonNULL >>>>>> is a real race that is not fixed >>>>>> >>>>>> Signed-off-by: Fred Isaman <iisaman@xxxxxxxxxx> >>>>>> --- >>>>>> fs/nfs/nfs4state.c | 5 +++-- >>>>>> fs/nfs/pnfs.c | 11 +++++++++++ >>>>>> include/linux/nfs4_pnfs.h | 2 ++ >>>>>> 3 files changed, 16 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c >>>>>> index d5144bd..8a7a64c 100644 >>>>>> --- a/fs/nfs/nfs4state.c >>>>>> +++ b/fs/nfs/nfs4state.c >>>>>> @@ -594,11 +594,12 @@ static void __nfs4_close(struct path *path, struct nfs4_state *state, >>>>>> } else { >>>>>> #ifdef CONFIG_NFS_V4_1 >>>>>> struct nfs_inode *nfsi = NFS_I(state->inode); >>>>>> + int roc = nfs4_roc_iomode(nfsi); >>>>>> >>>>>> - if (has_layout(nfsi) && nfsi->layout.roc_iomode) { >>>>>> + if (roc) { >>>>>> struct nfs4_pnfs_layout_segment range; >>>>>> >>>>>> - range.iomode = nfsi->layout.roc_iomode; >>>>>> + range.iomode = roc; >>>>>> range.offset = 0; >>>>>> range.length = NFS4_MAX_UINT64; >>>>>> pnfs_return_layout(state->inode, &range, NULL, >>>>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c >>>>>> index 6def09c..bd11ec7 100644 >>>>>> --- a/fs/nfs/pnfs.c >>>>>> +++ b/fs/nfs/pnfs.c >>>>>> @@ -321,6 +321,17 @@ pnfs_unregister_layoutdriver(struct pnfs_layoutdriver_type *ld_type) >>>>>> #define BUG_ON_UNLOCKED_LO(lo) do {} while (0) >>>>>> #endif /* CONFIG_SMP */ >>>>>> >>>>>> +int nfs4_roc_iomode(struct nfs_inode *nfsi) >>>>>> +{ >>>>>> + int rv = 0; >>>>>> + >>>>>> + spin_lock(&pnfs_spinlock); >>>>> >>>>> Why take the global lock rather than nfsi->lo_lock? >>>>> >>>>> Benny >>>> >>>> You are right. That would be a copy-paste error. >>> >>> What's an nfsi->lo_lock, and why do we need one? >>> >>> Trond >>> >>> >> >> It protects nfsi->layout and its contents. >> >> Fred > > Yes, but why do we need an extra spinlock? We already have > inode->i_lock. Why can't you just reuse that? > I agree. We can and should reuse i_lock. Benny -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html