Linux NFS development
 help / color / mirror / Atom feed
From: Tom Tucker <tom@opengridcomputing.com>
To: Neil Brown <neilb@suse.de>
Cc: nfs@lists.sourceforge.net, gnb@sgi.com
Subject: Re: [RFC, PATCH 06/33] svc: Add transport specific xpo_release	function
Date: Fri, 28 Sep 2007 11:06:04 -0500	[thread overview]
Message-ID: <1190995564.10604.44.camel@trinity.ogc.int> (raw)
In-Reply-To: <18172.28133.761225.855135@notabene.brown>

On Fri, 2007-09-28 at 12:58 +1000, Neil Brown wrote:
> On Thursday September 27, tom@opengridcomputing.com wrote:
> > 
> > The svc_sock_release function releases pages allocated to a thread. For
> > UDP, this also returns the receive skb to the stack. For RDMA it will 
> > post a receive WR and bump the client credit count. 
> > 
> ..
> > diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> > index 37f7448..cfb2652 100644
> > --- a/include/linux/sunrpc/svc.h
> > +++ b/include/linux/sunrpc/svc.h
> > @@ -217,7 +217,7 @@ struct svc_rqst {
> >  	struct auth_ops *	rq_authop;	/* authentication flavour */
> >  	u32			rq_flavor;	/* pseudoflavor */
> >  	struct svc_cred		rq_cred;	/* auth info */
> > -	struct sk_buff *	rq_skbuff;	/* fast recv inet buffer */
> > +	void *			rq_xprt_ctxt;	/* transport specific context ptr */
> >  	struct svc_deferred_req*rq_deferred;	/* deferred request we are replaying */
> >  
> >  	struct xdr_buf		rq_arg;
> ..
> > diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> > index cc8c7ce..e7d203a 100644
> > --- a/net/sunrpc/svcsock.c
> > +++ b/net/sunrpc/svcsock.c
> > @@ -184,14 +184,14 @@ svc_thread_dequeue(struct svc_pool *pool
> >  /*
> >   * Release an skbuff after use
> >   */
> > -static inline void
> > +static void
> >  svc_release_skb(struct svc_rqst *rqstp)
> >  {
> > -	struct sk_buff *skb = rqstp->rq_skbuff;
> > +	struct sk_buff *skb = (struct sk_buff *)rqstp->rq_xprt_ctxt;
> 
> Minor style point: We don't cast void* in the kernel.

Oops, no we don't.

> 
> >  	struct svc_deferred_req *dr = rqstp->rq_deferred;
> >  
> >  	if (skb) {
> > -		rqstp->rq_skbuff = NULL;
> > +		rqstp->rq_xprt_ctxt = NULL;
> >  
> >  		dprintk("svc: service %p, releasing skb %p\n", rqstp, skb);
> >  		skb_free_datagram(rqstp->rq_sock->sk_sk, skb);
> > @@ -394,7 +394,7 @@ svc_sock_release(struct svc_rqst *rqstp)
> >  {
> >  	struct svc_sock	*svsk = rqstp->rq_sock;
> >  
> > -	svc_release_skb(rqstp);
> > +	rqstp->rq_xprt->xpt_ops.xpo_release(rqstp);
> 
> These are somewhat ugly, aren't they?
> What would you think of giving rqstp a pointer directly to xpt_ops to
> avoid the double indirection?
> 

Maybe I'm missing something, but does it actually save anything? The
xpt_ops structure is copied into the transport instance. I think you'd
end up with this:

	rqstp->rq_ops->xpo_release(rqstp)

More aesthetically pleasing perhaps, but you'd still have "double
indirection". I think this is the same as we have today actually for the
sk_release/sk_sendto functions.

> NeilBrown


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs

  reply	other threads:[~2007-09-28 16:07 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-27  4:57 [RFC,PATCH 00/33] SVC Transport Switch Tom Tucker
2007-09-27  5:01 ` [RFC,PATCH 01/33] svc: Add an svc transport class Tom Tucker
2007-09-27  5:01 ` [RFC,PATCH 02/33] svc: Make svc_sock the tcp/udp transport Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 03/33] svc: Change the svc_sock in the rqstp structure to a transport Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 04/33] svc: Add a max payload value to the transport Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 05/33] svc: Move sk_sendto and sk_recvfrom to svc_xprt_class Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 06/33] svc: Add transport specific xpo_release function Tom Tucker
2007-09-28  2:58   ` Neil Brown
2007-09-28 16:06     ` Tom Tucker [this message]
2007-09-27  5:01 ` [RFC,PATCH 07/33] svc: Add per-transport delete functions Tom Tucker
2007-09-27  5:01 ` [RFC,PATCH 08/33] svc: Add xpo_prep_reply_hdr Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 09/33] svc: Add a transport function that checks for write space Tom Tucker
2007-09-28  3:03   ` Neil Brown
2007-09-28 16:09     ` Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 10/33] svc: Move close processing to a single place Tom Tucker
2007-09-27  5:01 ` [RFC,PATCH 11/33] svc: Add xpo_accept transport function Tom Tucker
2007-09-28  3:21   ` Neil Brown
2007-09-28 16:10     ` Tom Tucker
2007-09-27  5:01 ` [RFC, PATCH 12/33] svc: Add a generic transport svc_create_xprt function Tom Tucker
2007-09-28  3:21   ` Neil Brown
2007-09-28 16:15     ` Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 13/33] svc: Change services to use new svc_create_xprt service Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 14/33] svc: Change sk_inuse to a kref Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 15/33] svc: Move sk_flags to the svc_xprt structure Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 16/33] svc: Move sk_server and sk_pool to svc_xprt Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 17/33] svc: Make close transport independent Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 18/33] svc: Move sk_reserved to svc_xprt Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 19/33] svc: Make the enqueue service transport neutral and export it Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 20/33] svc: Make svc_send transport neutral Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 21/33] svc: Change svc_sock_received to svc_xprt_received and export it Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 22/33] svc: Move sk_lastrecv to svc_xprt Tom Tucker
2007-09-28  4:25   ` Neil Brown
2007-09-28 16:16     ` Tom Tucker
2007-10-03 10:10       ` Greg Banks
2007-09-27  5:02 ` [RFC,PATCH 23/33] svc: Move the authinfo cache " Tom Tucker
2007-09-28  4:30   ` [RFC, PATCH " Neil Brown
2007-09-29 20:49     ` Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 24/33] svc: Make deferral processing xprt independent Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 25/33] svc: Move the sockaddr information to svc_xprt Tom Tucker
2007-09-28  4:36   ` Neil Brown
2007-09-28 16:44     ` Tom Tucker
2007-09-28 16:53       ` Chuck Lever
2007-09-27  5:02 ` [RFC, PATCH 26/33] svc: Make svc_sock_release svc_xprt_release Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 27/33] svc: Make svc_recv transport neutral Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 28/33] svc: Make svc_age_temp_sockets svc_age_temp_transports Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 29/33] svc: Move common create logic to common code Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 30/33] svc: Removing remaining references to rq_sock in rqstp Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 31/33] svc: Move the xprt independent code to the svc_xprt.c file Tom Tucker
2007-09-27  5:02 ` [RFC,PATCH 32/33] svc: Add /proc/sys/sunrpc/transport files Tom Tucker
2007-09-27  5:02 ` [RFC, PATCH 33/33] knfsd: Support adding transports by writing portlist file Tom Tucker
2007-09-28  4:48   ` Neil Brown
2007-09-28 17:33     ` Tom Tucker
2007-10-03 10:27     ` Greg Banks
2007-09-27 17:55 ` [RFC,PATCH 00/33] SVC Transport Switch J. Bruce Fields
2007-09-28  4:51 ` Neil Brown
2007-09-28 17:39   ` Tom Tucker
2007-10-01 15:58     ` J. Bruce Fields
2007-10-03  9:57 ` Greg Banks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1190995564.10604.44.camel@trinity.ogc.int \
    --to=tom@opengridcomputing.com \
    --cc=gnb@sgi.com \
    --cc=neilb@suse.de \
    --cc=nfs@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox