Re: [PATCH 07/10] pnfs-submit: avoid race handling return on close

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

--
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


[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux