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