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. > IOW: if both objects and blocks track the information that they need for > layoutcommit outside the layout segments, then why do we need the > NFS_LSEG_LAYOUTCOMMIT bit and pls_lc_list at all? If we remove NFS_LSEG_LAYOUTCOMMIT bit and pls_lc_list, we must find other method to protect agains freeing lseg while layoutcommit is needed or is going on. e.g., check for NFS_INO_LAYOUTCOMMIT bit and inflight layoutcommit in initiate_file_draining(). Thanks, Tao ��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
