lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 11 Mar 2013 15:15:45 -0400
From:	"J. Bruce Fields" <bfields@...ldses.org>
To:	Tim Gardner <tim.gardner@...onical.com>
Cc:	linux-kernel@...r.kernel.org,
	Trond Myklebust <Trond.Myklebust@...app.com>,
	"David S. Miller" <davem@...emloft.net>, Tom Tucker <tom@....us>,
	Haggai Eran <haggaie@...lanox.com>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Shani Michaeli <shanim@...lanox.com>,
	linux-nfs@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH linux-next v2] SUNRPC: rpcrdma_register_default_external:
 Dynamically allocate ib_phys_buf

On Mon, Mar 11, 2013 at 12:51:44PM -0600, Tim Gardner wrote:
> 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).

Right, though I think it shouldn't be GFP_KERNEL--looks like writes
could wait on it.

In any case, the embedding-in-rpcrdma_req solution does look cleaner if
that's correct (e.g. if we can be sure there won't be two simultaneous
users of that array).

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists