[pnfs] [PATCH 19/20] 2.6-latest pnfs client no rpc nfs page cache cleanup

[William A. (Andy) Adamson]

On 12/2/07, Dean Hildebrand <seattleplus at gmail.com> wrote:
>
>
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index aa40ab2..27b3be5 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -2944,6 +2944,10 @@ static int pnfs4_read_done(struct rpc_task *task,
> struct nfs_read_data *data)
> >       int status;
> >
> >       dprintk("--> %s\n", __func__);
> > +
> > +     if (data->pnfsflags & PNFS_NO_RPC)
> > +             return 0;
> > +
> >       status = task->tk_status >= 0 ? 0 : task->tk_status;
> >
> >       /* Is this a DS session */
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index b072ec0..a7d47bc 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -916,6 +916,8 @@ out:
> >
> >  /* Post-read completion function.  Invoked by non RPC layout drivers
> >   * to clean up read pages.
> > + *
> > + * NOTE: called must set data->pnfsflags PNFS_NO_RPC
> >   */
> >  static void
> >  pnfs_read_done(struct nfs_read_data* data, ssize_t status, int eof)
> > @@ -927,12 +929,17 @@ pnfs_read_done(struct nfs_read_data* data, ssize_t
> status, int eof)
> >           pnfs_use_nfsv4_rproto(data->inode, data->args.count))
> >               return;
> >
> > -     /* Status is the number of bytes written or an error code */
> > +     /* Status is the number of bytes written or an error code
> > +      * the rpc_task is uninitialized, and tk_status is all that
> > +      * is used in the call done routines.
> > +     */
> >       data->task.tk_status = status;
> >       data->res.eof = eof;
> >       data->res.count = status;
> > -     pnfs_readpage_result_norpc(&data->task, data);
> > -     nfs_readdata_release(data);
> > +
> > +     /* call the NFS cleanup routines. */
> > +     data->call_ops->rpc_call_done(&data->task, data);
> > +     data->call_ops->rpc_release(data);
> >  }
> >
> I think Benny was hinting at this, and I'm not sure if I get it
> completely yet, but it seems that the line above:
>
> data->call_ops->rpc_call_done(&data->task, data);
>
> evaluates to
>
> pnfs4_read_done
>
> which then exists immediately because PNFS_NO_RPC must be set. Is this
> true?


no - false! once again:

data->call_ops->rpc_call_done() evaluates to either

nfs_readpage_result_full()

or

nfs_readpage_result_partial()

each of which calls

nfs_readpage_result() which calls NFS_PROTO(inode)->read_done, and then does
some page cache cleanup.

and it's the NFS_PROTO(inode)->read_done which evaluates to

pnfs4_read_done.

the no-rpc path wants to clean up the page cache in exactly the same way
that the rpc path does.

nfs_readpage_result_full() cleans up the nfs page cache differently than
nfs_readpage_result_partial()

in the 2.6.28.3 code, i copied the page cleanup code from
nfs_readpage_result_full() and nfs_readpage_result_partial() into the
norpc_pnfs_readpage_result().

in the 2.6.24-rc3 code, set the PNFS_NO_RPC flag and call the
nfs_readpage_result_full() or nfs_readpage_result_partial() code.


here is what the last message said:

 so pnfs_read_done calls data->call_ops->rpc_call_done() which is either
nfs_readpage_result_full() or nfs_readpage_result_partial()
both of these functions call nfs_readpage_result() which in turn calls
NFS_PROTO(data->inode)->read_done() which is pnfs4_read_done, which does the
right thing for the NO_RPC path when the PNFS_NO_RPC flag is set.


If so, just remove the call to the function.


nope. then there would be no nfs page cache cleanup on the no-rpc path.

-->Andy

Dean
> >
> >  /*
> > diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> > index 4cf47c5..1852434 100644
> > --- a/fs/nfs/read.c
> > +++ b/fs/nfs/read.c
> > @@ -381,6 +381,10 @@ static int nfs_readpage_retry(struct rpc_task
> *task, struct nfs_read_data *data)
> >       struct nfs_readargs *argp = &data->args;
> >       struct nfs_readres *resp = &data->res;
> >
> > +#ifdef CONFIG_PNFS
> > +     if (data->pnfsflags & PNFS_NO_RPC)
> > +             return 0;
> > +#endif /* CONFIG_PNFS */
> >       if (resp->eof || resp->count == argp->count)
> >               return 0;
> >
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index a6f429b..f747637 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -1069,7 +1069,7 @@ struct nfs_read_data {
> >  #ifdef CONFIG_PNFS
> >  /* pnfsflag values */
> >  #define PNFS_ISSYNC          0x0001   /* sync I/O request */
> > -#define PNFS_USE_FULL_CB     0x0002   /* non rpc result callback switch
> */
> > +#define PNFS_NO_RPC          0x0002   /* non rpc result callback switch
> */
> >  #endif /* CONFIG_PNFS */
> >
> >  struct nfs_write_data {
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20071203/760211c3/attachment.htm