[pnfs] Status meeting pNFS

Benny Halevy bhalevy at panasas.com
Tue Oct 24 04:08:56 EDT 2006


please put the following on the agenda:

1) bug in layoutreturn implementation:
the "type" field in layoutreturn_arg was misinterpreted as layout type 
rather
than layoutreturn_type (LAYOUTRETURN_{FILE,FSID,ALL}).
The patch to fix that looks like this (I renamed some fields and 
reshuffled their
order in the in-core structure to resemble the on-the-wire order)

--- /tmp/tmp.1856.7     2006-10-24 09:44:45.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/include/linux/pnfs_xdr.h    
2006-10-24 00:16:49.000000000 +0200
@@ -89,13 +89,14 @@ struct pnfs_layoutcommit_data {
 };

 struct nfs4_pnfs_layoutreturn_arg {
+       struct inode* inode;
        __u64   clientid;
+       __u32   reclaim;
+       __u32   layout_type;
+       __u32   iomode;
+       __u32   return_type;
        __u64   offset;
        __u64   length;
-       __u32   iomode;
-       __u32   reclaim;
-       __u32   type;
-       struct inode* inode;
 };

 struct nfs4_pnfs_layoutreturn {
--- /tmp/tmp.1856.6     2006-10-24 09:44:45.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/include/linux/nfsd/nfsd4_pnfs.h     
2006-10-23 18:35:46.000000000 +0200
@@ -110,12 +110,12 @@ struct nfsd4_pnfs_layoutcommit {

 struct nfsd4_pnfs_layoutreturn {
        u64                             lr_clientid;    /* request */
+       u32                             lr_reclaim;     /* request */
+       u32                             lr_layout_type; /* request */
+       u32                             lr_iomode;      /* request */
+       u32                             lr_return_type; /* request */
        u64                             lr_offset;      /* request */
        u64                             lr_length;      /* request */
-       u32                             lr_iomode;      /* request */
-       u32                             lr_file;        /* request */
-       u32                             lr_reclaim;     /* request */
-       u32                             lr_type;        /* request */
 };

 struct layout_return {
--- /tmp/tmp.1856.1     2006-10-24 09:44:41.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/fs/nfs/nfs4xdr.c    2006-10-23 
18:20:08.000000000 +0200
@@ -4824,10 +4824,10 @@ static int encode_pnfs_layoutreturn(stru
        WRITE32(OP_LAYOUTRETURN);
        WRITE64(args->clientid);
        WRITE32(args->reclaim);
-       WRITE32(args->type);
+       WRITE32(args->layout_type);
        WRITE32(args->iomode);
-       WRITE32(LAYOUTRETURN_FILE);
-       if (args->type == LAYOUT_NFSV4_FILES)
+       WRITE32(args->return_type);
+       if (args->return_type == LAYOUTRETURN_FILE)
        {
                WRITE64(args->offset);
                WRITE64(args->length);
--- /tmp/tmp.1856.2     2006-10-24 09:44:41.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/fs/nfs/pnfs.c       2006-10-24 
01:32:42.000000000 +0200
@@ -342,14 +344,15 @@ pnfs_return_layout(struct inode* ino)

        dprintk("%s: Returning layout...\n", __FUNCTION__);

+       arg.inode = ino;
        arg.clientid = server->nfs4_state->cl_clientid;
-       arg.type = server->pnfs_curr_ld->id;
+       arg.reclaim = 0;
+       arg.layout_type = server->pnfs_curr_ld->id;
        /* FMODE_WRITE =2= LAYOUTMODE_RW,  FMODE_READ =1= LAYOUTMODE_READ */
        arg.iomode = FMODE_WRITE;
+       arg.return_type = LAYOUTRETURN_FILE;
        arg.offset = 0;
-       arg.reclaim = 0;
        arg.length = ~0;
-       arg.inode = ino;

        /* Retrieve layout information from server */
        status = NFS_PROTO(ino)->pnfs_layoutreturn(&gdata);
--- /tmp/tmp.2095.4     2006-10-24 09:57:27.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/fs/nfsd/nfs4proc.c  2006-10-23 
18:39:00.000000000 +0200
@@ -924,9 +924,9 @@ nfsd4_layoutreturn( struct svc_rqst *rqs
        /* check to see if requested layout type is supported. */
        status = nfserr_unknown_layouttype;
        if (!sb->s_export_op->layout_type ||
-               ((type = sb->s_export_op->layout_type()) != lrp->lr_type)) {
+               ((type = sb->s_export_op->layout_type()) != 
lrp->lr_layout_type)) {

-               printk("pNFS %s: requested layout type %d does not match 
suppored type %d\n", __FUNCTION__, lrp->lr_type, type);
+               printk("pNFS %s: requested layout type %d does not match 
suppored type %d\n", __FUNCTION__, lrp->lr_layout_type, type);
                goto out;
        }
--- /tmp/tmp.2095.5     2006-10-24 09:57:27.000000000 +0200
+++ /home/bhalevy/p4.local/pnfs-dev/fs/nfsd/nfs4xdr.c   2006-10-23 
18:37:03.000000000 +0200
@@ -1114,10 +1114,10 @@ nfsd4_decode_layoutreturn(struct nfsd4_c
        READ_BUF(40);
        READ64(lrp->lr_clientid);
        READ32(lrp->lr_reclaim);
-       READ32(lrp->lr_type);
+       READ32(lrp->lr_layout_type);
        READ32(lrp->lr_iomode);
-       READ32(lrp->lr_file);
-       if (lrp->lr_type == LAYOUT_NFSV4_FILES)
+       READ32(lrp->lr_return_type);
+       if (lrp->lr_return_type == LAYOUTRETURN_FILE)
        {
                READ64(lrp->lr_offset);
                READ64(lrp->lr_length);


2) layout commit
a. there are a couple of places on the writepages path (both sync and 
async) in which
   I think we should update the file length and mark the file for 
layoutcommit.
  I'm still working on this but it looks like we need to add the 
following to pnfs_writepages
  on the synchronous return path:
     status = 
nfss->pnfs_curr_ld->ld_io_ops->write_pagelist(nfsi->current_layout,
                                    inode,
                                    args->pages,
                                    args->pgbase,
                                    numpages,
                                    (loff_t)args->offset,
                                    args->count,
                                    how,
                                    wdata);
 
+    if (status > 0) {
+        pnfs_update_last_write(nfsi, args->offset, status);
+        pnfs_need_layoutcommit(nfsi, NULL /*(struct nfs_open_context 
*)filp->private_data*/);
+    }
+
out:

and in pnfs_writeback_done_update(), the call to pnfs_need_layoutcommit
is conditional and depends on whether the data is already on stable storage.
I think it should be unconditional for other layout types...

-               if (pnfs_use_nfsv4_wproto(data->inode, argp->count) &&
+               if (!pnfs_use_nfsv4_wproto(data->inode, argp->count) ||
                    ((resp->verf->committed == NFS_FILE_SYNC)  ||
                    (data->how == FLUSH_STABLE && data->call_ops == NULL)))
                        pnfs_need_layoutcommit(nfsi, argp->context);

b. I'd like to add a layout_commit() entry point to the server side 
export_operations
so that the server could implement layuot_commit appropriately and parse 
the layout_type
specific layoutupdate4.

c. Currently nfsd4_layoutcommit() is generating a setattr call into the
file system via notify_change.  This is problematic for us since at 
layoutcommit
time we have an outstanding write layout at the pnfs client so we want 
to just
update the attributes in cache and flush later when the layout is returned.
What do you think the best way to do that?  Do the change_notify at the
file system layout_commit function or implement inode_operations->setattr
differently for the pnfs daemon?

3) compiler warnings in pnfs.c
some printf format strings need to change fromr %d to %Zd for ssize_t 
parameters.

Benny

Marc Eshel wrote:
> Status meeting on Wednesday 10/25/06 at 10am pacific time (1pm UMICH time)
>  
> Let me know if you need the new phone number for the conference call.
>
> Marc. 
>
>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>   



More information about the pNFS mailing list