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: Benny Halevy [mailto:bhalevy@xxxxxxxxxx]
> Sent: Tuesday, September 13, 2011 4:32 AM
> To: Peng Tao
> Cc: Trond Myklebust; Peng, Tao; gusev.vitaliy@xxxxxxxxxxx;
> gusev.vitaliy@xxxxxxxxx; linux-nfs@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release
> 
> On 2011-09-12 07:56, Peng Tao wrote:
> > On Sat, Sep 10, 2011 at 3:14 PM, Benny Halevy <bhalevy@xxxxxxxxxx> wrote:
> >> On 2011-09-09 11:20, Trond Myklebust wrote:
> >>> On Thu, 2011-09-08 at 23:11 -0400, tao.peng@xxxxxxx wrote:
> >>>> HI, Trond,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Myklebust, Trond [mailto:Trond.Myklebust@xxxxxxxxxx]
> >>>>> Sent: Friday, September 09, 2011 1:05 AM
> >>>>> To: Peng Tao
> >>>>> Cc: Peng, Tao; gusev.vitaliy@xxxxxxxxxxx; gusev.vitaliy@xxxxxxxxx;
> >>>>> linux-nfs@xxxxxxxxxxxxxxx
> >>>>> Subject: RE: [PATCH] nfs: fix inifinite loop at nfs4_layoutcommit_release
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Peng Tao [mailto:bergwolf@xxxxxxxxx]
> >>>>>> Sent: Thursday, September 08, 2011 11:00 AM
> >>>>>> To: Myklebust, Trond
> >>>>>> Cc: tao.peng@xxxxxxx; gusev.vitaliy@xxxxxxxxxxx;
> >>>>>> gusev.vitaliy@xxxxxxxxx; linux-nfs@xxxxxxxxxxxxxxx
> >>>>>> Subject: Re: [PATCH] nfs: fix inifinite loop at
> >>>>>> nfs4_layoutcommit_release
> >>>>>>
> >>>>>> On Thu, Sep 8, 2011 at 8:01 PM, Myklebust, Trond
> >>>>>> <Trond.Myklebust@xxxxxxxxxx> wrote:
> >>>>>>>> -----Original Message-----
> >>>>>>
> >>>>>>>
> >>>>>>> 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.
> >>>>>>
> >>>>>> blocklayout does not decide what to layoutcommit according to the lseg
> >>>>>> list. Instead, it keeps track of each extent's state at the
> >>>>>> granularity of blocksize, and encode whatever needs layoutcommitted in
> >>>>>> the layoutcommit call. So in your above case, as long as the second
> >>>>>> layoutcommit is issued, blocklayout will encode the newly written
> >>>>>> extent in the second layoutcommit call, even if the lseg is not
> >>>>>> attached to the second layoutcommit.
> >>>>>>
> >>>>>> But that leads to another two question:
> >>>>>> 1. How do we protect against layoutrecall if lseg is not linked to
> >>>>>> layoutcommit? For this one, can we just reject layoutrecall if there
> >>>>>> is inflight layoutcommit? It will be less parallel but can guarantee
> >>>>>> current implementation correctness.
> >>>>>> 2. blocklayout ONLY: bl_committing may be overloaded by several
> >>>>>> layoutcommit calls and we don't have information in
> >>>>>> cleanup_layoutcommit() on how many entry should be removed from
> >>>>>> bl_committing. Maybe we can add a (void*) to struct
> >>>>>> nfs4_layoutcommit_data, so that LD can pass some private information
> >>>>>> between encode_layoutcommit() and cleanup_layoutcommit()?
> >>>>>
> >>>>> 3. What is the purpose of pinning the layout segment at all if neither blocks,
> nor
> >>>>> objects nor files cares?
> >>>> I believe it is for protecting against layoutrecall. But since we are seperating
> lseg and LD specific layout information management, it is actually not working as
> expected.
> >>>>
> >>
> >> The layout segments are not really in use while in LAYOUTCOMMIT.
> >> We only need to get the stateid right with respect to concurrent layout recalls.
> > LAYOUTCOMMIT takes lseg reference to mark them as in use so that
> > layoutrecall cannot free them.
> >
> 
> And if layoutrecall would have freed layout segments during layoutcommit,
> what is your specific concern?
In layoutcommit_release, blocklayout need to access the corresponding extents to convert their states. If the layout segments are freed by layoutrecall, it can cause problems.

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


[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