On 2011-09-13 00:02, tao.peng@xxxxxxx wrote: > >> -----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. > See my response to Trond on his previous message. I think the best thing to do is to return NFS4ERR_DELAY if the gets a conflicting CB_LAYOUTRECALL while the LAYOUTCOMMIT is in progress. The server may need to reject the LAYOUTCOMMIT in this case to prevent a distributed deadlock so the client should be prepared to retry. Benny >> >> 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
