[pnfs] CVS: nfsv4

[Marc Eshel]

Benny Halevy <bhalevy at panasas.com> wrote on 12/10/2006 08:08:26 AM:

> Marc Eshel wrote:
> > Hi Benny,
> > I am not sure that this is a bug. So you want to share the same layout 

> > struct on the server for all clients ? That might be more difficult to 

> > keep track of which client has which byte range. I had a layout struct 
per 
> > client which is also on the file struct. So if you need to see all 
layouts 
> > per client you run the chain of the client struct and if you need to 
see 
> > all the layouts per file you run the chain of the file struct.
> > Marc.
> 
> Yes, we need a more efficient way to track the layouts eventually
> but now the while() expression checked the client list while
> the list operation is on the file's list.  The client had a layout
> for a different file while the file's layout list was empty...
 
That is why I had the comment "//??? if the right layout". I agree that 
without that check it was broken but the change you made did not fix it. 
Right now when a second request for a layout arrives from the 2nd client 
for the same file the 
        while (!list_empty(&fp->fi_layouts)) {
                lp = list_entry(fp->fi_layouts.next, 
will find it and increment the hold count on lp but it will not add it to 
the 2nd client chain, so when purging the 2nd client you will find nothing 
on its chain to recall. The layout is on the file chain and the first 
client that requested this layout. 

> I completely agree that you should either check against the per-client
> list and compare the file and byte range, or check against the
> per-file list and match the client and byte range. I'm not sure
> what will be more efficient though...

I had a separate layout struct for each client per file. You changed it to 
have one layout for all clients per file. I don't think that this is the 
right thing to do but I will not make any more changes until we talk on 
Wednesday meeting and decide how to do it.

> 
> If we keep a counter on both lists we can make the decision in run-time,
> however, since  file sharing is less common I believe that the per-file
> list should typically be shorter than or equal to the per-client list.
> Having an efficient hash table rather than a list should help both cases 
:)
> 
> > 
> > 
> > pnfs-bounces at linux-nfs.org wrote on 12/07/2006 11:06:01 PM:
> > 
> >> CVSROOT:   /cvs
> >> Module name:   nfsv4
> >> Changes by:   bhalevy at citi.   2006/12/08 02:06:01
> >>
> >> Modified files:
> >>    cvs/pnfs/fs/nfsd: nfs4state.c 
> >>
> >> Log message:
> >> Fix bug in nfs4_pnfs_get_layout()
> >>
> >> Index: fs/nfsd/nfs4state.c
> >> ===================================================================
> >> RCS file: /cvs/nfsv4/cvs/pnfs/fs/nfsd/nfs4state.c,v
> >> retrieving revision 1.21
> >> diff -u -p -r1.21 nfs4state.c
> >> --- fs/nfsd/nfs4state.c   6 Dec 2006 23:18:02 -0000   1.21
> >> +++ fs/nfsd/nfs4state.c   8 Dec 2006 07:05:03 -0000
> >> @@ -3551,7 +3551,7 @@ int nfs4_pnfs_get_layout(struct super_bl
> >> goto out;
> >>
> >> lgp->lg_flags = 0;
> >> -   while (!list_empty(&clp->cl_layouts)) {
> >> +   while (!list_empty(&fp->fi_layouts)) {
> >> lp = list_entry(fp->fi_layouts.next,
> >> struct nfs4_layout, lo_perfile);
> >> //??? if the right layout
> >>
> >> _______________________________________________
> >> pNFS mailing list
> >> pNFS at linux-nfs.org
> >> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> > 
> > _______________________________________________
> > pNFS mailing list
> > pNFS at linux-nfs.org
> > http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>