RE: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release

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

 



> -----Original Message-----
> From: tao.peng@xxxxxxx [mailto:tao.peng@xxxxxxx]
> Sent: Thursday, September 08, 2011 6:21 AM
> To: Myklebust, Trond; gusev.vitaliy@xxxxxxxxxxx
> Cc: gusev.vitaliy@xxxxxxxxx; linux-nfs@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH] nfs: fix inifinite loop at
> nfs4_layoutcommit_release
> 
> Hi, Trond,
> 
> Sorry for the late response.
> 
> > -----Original Message-----
> > From: Trond Myklebust [mailto:Trond.Myklebust@xxxxxxxxxx]
> > Sent: Wednesday, September 07, 2011 6:33 AM
> > To: Vitaliy Gusev
> > Cc: Vitaliy Gusev; Peng, Tao; linux-nfs@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH] nfs: fix inifinite loop at
> nfs4_layoutcommit_release
> >
> > On Wed, 2011-09-07 at 02:13 +0400, Vitaliy Gusev wrote:
> > > >> @@ -1376,7 +1376,8 @@ static void pnfs_list_write_lseg(struct
> inode *inode,
> > struct list_head *listp)
> > > >>
> > > >>   	list_for_each_entry(lseg,&NFS_I(inode)->layout->plh_segs,
> pls_list) {
> > > >>   		if (lseg->pls_range.iomode == IOMODE_RW&&
> > > >> -		    test_bit(NFS_LSEG_LAYOUTCOMMIT,&lseg->pls_flags))
> > > >> +		    test_bit(NFS_LSEG_LAYOUTCOMMIT,&lseg-
> >pls_flags)&&
> > > >> +		    list_empty(&lseg->pls_lc_list))
> > > >>   			list_add(&lseg->pls_lc_list, listp);
> > > >>   	}
> > > >>   }
> > > >
> > > > If the lseg is already part of one layoutcommit, but we're
> sending a
> > > > second one for the same range (presumably because we wrote more
> data in
> > > > the same region), then the above causes the lseg to be excluded.
> > >
> > >
> > > Yes, lseg is excluded, This patch does exactly only exclusion of
> lseg.
> > > lseg is used here only to get/put reference on this lseg, so
> skipping is
> > > correct.
> >
> > Are you sure? As far as I can see, pnfs_list_write_lseg() actually
> > assigns the lseg to a particular layoutcommit call.
> >
> > My questions are: if I write to an area described by that lseg
> _after_
> > it has been assigned to that layoutcommit RPC call (and the latter
> has
> > already been dispatched to the metadata server), then
> >      A. shouldn't it be assigned to a second layoutcommit call too?
> >      B. If not, what are we doing to ensure mutual exclusion between
> >         layoutcommit RPC calls and pNFS writes to the data server?
> I think it depends on the purpose of layoutcommit.
> 1. for updating atime/mtime. In this case, we don't need mutual
> exclusion

Agreed.

> between layoutcommit RPC and pNFS writes to data server.
> 2. for updating LD specific state. In this case, LD should decide what
> to commit
> (and actually ignores the range of lseg). Take block layout for
> example. It maintains
> blocksized extent state inside LD and determines what to encode inside
> layoutcommit
> RPC whenever there is a generic layer layoutcommit call. So when an
> extent is being
> layoutcommitted, client can still write to the same range, as long as
> the lseg is held by client.

Yes, but as far as I can see, even in the blocks case there can be multiple extents per layout segment. What if I write to one uninitialised extent, layoutcommit, then write to another uninitialized extent in the same layout segment and layoutcommit? In my reading of the code, there is a chance that the second layoutcommit will fail to pick up the layout segment, and so will fail to notify the MDS that the second extent now contains data.

Trond
��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥



[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