All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tim Gardner <tim.gardner@canonical.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: linux-kernel@vger.kernel.org,
	Trond Myklebust <Trond.Myklebust@netapp.com>,
	"David S. Miller" <davem@davemloft.net>, Tom Tucker <tom@ogc.us>,
	Haggai Eran <haggaie@mellanox.com>,
	Or Gerlitz <ogerlitz@mellanox.com>,
	Shani Michaeli <shanim@mellanox.com>,
	linux-nfs@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
Date: Mon, 11 Mar 2013 12:51:44 -0600	[thread overview]
Message-ID: <513E27C0.8050108@canonical.com> (raw)
In-Reply-To: <20130311181450.GC30618@fieldses.org>

On 03/11/2013 12:14 PM, J. Bruce Fields wrote:
<snip>
>>
>> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
>> and pass this request down through rpcrdma_register_external() and
>> rpcrdma_register_default_external(). This is less overhead then using
>> kmalloc() and requires no extra error checking as the allocation burden is
>> shifted to the transport client.
> 
> Oh good--so that works, and the req is the right place to put this?  How
> are you testing this?
> 
> (Just want to make it clear: I'm *not* an expert on the rdma code, so my
> suggestion to put this in the rpcrdma_req was a suggestion for something
> to look into, not a claim that it's correct.)
> 

Just compile tested so far. Incidentally, I've been through the call stack:

call_transmit
 xprt_transmit
  xprt->ops->send_request(task)
   xprt_rdma_send_request
    rpcrdma_marshal_req
     rpcrdma_create_chunks
      rpcrdma_register_external
       rpcrdma_register_default_external

It appears that the context for kmalloc() should be fine unless there is
a spinlock held around call_transmit() (which seems unlikely).

rtg
-- 
Tim Gardner tim.gardner@canonical.com

WARNING: multiple messages have this Message-ID (diff)
From: Tim Gardner <tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
To: "J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Trond Myklebust
	<Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>,
	"David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	Tom Tucker <tom-/Yg/VP3ZvrM@public.gmane.org>,
	Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	Shani Michaeli <shanim-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf
Date: Mon, 11 Mar 2013 12:51:44 -0600	[thread overview]
Message-ID: <513E27C0.8050108@canonical.com> (raw)
In-Reply-To: <20130311181450.GC30618-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

On 03/11/2013 12:14 PM, J. Bruce Fields wrote:
<snip>
>>
>> v2 - Move the array of 'struct ib_phys_buf' objects into struct rpcrdma_req
>> and pass this request down through rpcrdma_register_external() and
>> rpcrdma_register_default_external(). This is less overhead then using
>> kmalloc() and requires no extra error checking as the allocation burden is
>> shifted to the transport client.
> 
> Oh good--so that works, and the req is the right place to put this?  How
> are you testing this?
> 
> (Just want to make it clear: I'm *not* an expert on the rdma code, so my
> suggestion to put this in the rpcrdma_req was a suggestion for something
> to look into, not a claim that it's correct.)
> 

Just compile tested so far. Incidentally, I've been through the call stack:

call_transmit
 xprt_transmit
  xprt->ops->send_request(task)
   xprt_rdma_send_request
    rpcrdma_marshal_req
     rpcrdma_create_chunks
      rpcrdma_register_external
       rpcrdma_register_default_external

It appears that the context for kmalloc() should be fine unless there is
a spinlock held around call_transmit() (which seems unlikely).

rtg
-- 
Tim Gardner tim.gardner-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2013-03-11 18:51 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10 15:39 [PATCH linux-next] SUNRPC: rpcrdma_register_default_external: Dynamically allocate ib_phys_buf Tim Gardner
2013-03-10 15:39 ` Tim Gardner
2013-03-10 17:16 ` Tom Tucker
2013-03-10 17:16   ` Tom Tucker
2013-03-10 18:20   ` Tim Gardner
2013-03-10 20:28 ` J. Bruce Fields
2013-03-10 20:28   ` J. Bruce Fields
2013-03-11 17:37   ` [PATCH linux-next v2] " Tim Gardner
2013-03-11 17:37     ` Tim Gardner
2013-03-11 18:14     ` J. Bruce Fields
2013-03-11 18:14       ` J. Bruce Fields
2013-03-11 18:51       ` Tim Gardner [this message]
2013-03-11 18:51         ` Tim Gardner
2013-03-11 19:15         ` J. Bruce Fields
2013-03-11 19:48           ` Myklebust, Trond
2013-03-11 19:48             ` Myklebust, Trond
2013-03-11 20:00             ` J. Bruce Fields
2013-03-11 20:00               ` J. Bruce Fields
2013-03-11 21:15             ` [PATCH linux-next v3] " Tim Gardner
2013-03-11 21:25               ` J. Bruce Fields
2013-03-11 21:25                 ` J. Bruce Fields
2013-03-11 23:02                 ` Tom Tucker
2013-03-11 23:02                   ` Tom Tucker
2013-03-12  2:53                   ` Tim Gardner
2013-03-12  3:40                     ` Tom Tucker
2013-03-12  3:40                       ` Tom Tucker

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=513E27C0.8050108@canonical.com \
    --to=tim.gardner@canonical.com \
    --cc=Trond.Myklebust@netapp.com \
    --cc=bfields@fieldses.org \
    --cc=davem@davemloft.net \
    --cc=haggaie@mellanox.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ogerlitz@mellanox.com \
    --cc=shanim@mellanox.com \
    --cc=tom@ogc.us \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.