Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-20 Thread Sagi Grimberg
On 4/19/2014 7:31 PM, Chuck Lever wrote: Hi Sagi- On Apr 17, 2014, at 3:11 PM, Sagi Grimberg wrote: On 4/17/2014 5:34 PM, Steve Wise wrote: You could use a small array combined with a loop and a budget count. So the code would grab, say, 4 at a time, and keep looping polling up to 4 unti

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-19 Thread Chuck Lever
Hi Sagi- On Apr 17, 2014, at 3:11 PM, Sagi Grimberg wrote: > On 4/17/2014 5:34 PM, Steve Wise wrote: > > >> You could use a small array combined with a loop and a budget count. So the >> code would >> grab, say, 4 at a time, and keep looping polling up to 4 until the CQ is >> empty or the >

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-17 Thread Sagi Grimberg
On 4/17/2014 5:34 PM, Steve Wise wrote: You could use a small array combined with a loop and a budget count. So the code would grab, say, 4 at a time, and keep looping polling up to 4 until the CQ is empty or the desired budget is reached... Bingo... couldn't agree more. Poll Arrays are a

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-17 Thread Sagi Grimberg
On 4/17/2014 4:55 PM, Chuck Lever wrote: On Apr 17, 2014, at 3:06 AM, Sagi Grimberg wrote: On 4/16/2014 9:21 PM, Chuck Lever wrote: Passing a small array to ip_poll_cq() is actually easy to do, and is exactly equivalent to a poll budget. The struct ib_wc should be taken off the stack anyway,

RE: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-17 Thread Steve Wise
t; Subject: Re: [PATCH 7/8] xprtrdma: Split the completion queue > > > On Apr 17, 2014, at 3:06 AM, Sagi Grimberg wrote: > > > On 4/16/2014 9:21 PM, Chuck Lever wrote: > >> Passing a small array to ip_poll_cq() is actually easy to do, and is > >> exactly e

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-17 Thread Chuck Lever
On Apr 17, 2014, at 3:06 AM, Sagi Grimberg wrote: > On 4/16/2014 9:21 PM, Chuck Lever wrote: >> Passing a small array to ip_poll_cq() is actually easy to do, and is >> exactly equivalent to a poll budget. The struct ib_wc should be taken >> off the stack anyway, IMO. >> >> The only other exampl

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-17 Thread Sagi Grimberg
On 4/16/2014 9:21 PM, Chuck Lever wrote: Passing a small array to ip_poll_cq() is actually easy to do, and is exactly equivalent to a poll budget. The struct ib_wc should be taken off the stack anyway, IMO. The only other example I see in 3.15 right now is IPoIB, which seems to do exactly this.

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Chuck Lever
On Apr 16, 2014, at 11:23 AM, Sagi Grimberg wrote: > On 4/16/2014 6:08 PM, Chuck Lever wrote: >> Hi Sagi- >> >> Thanks for the review! In-line replies below. > >> 2. If I understood correctly, I see that the CQs are loop-polled in an interrupt context. This may become probl

RE: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Steve Wise
> -Original Message- > From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il] > Sent: Wednesday, April 16, 2014 10:18 AM > To: Steve Wise; 'Chuck Lever'; linux-...@vger.kernel.org; > linux-rdma@vger.kernel.org > Subject: Re: [PATCH 7/8] xprtrdma: Split the c

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Sagi Grimberg
On 4/16/2014 6:08 PM, Chuck Lever wrote: Hi Sagi- Thanks for the review! In-line replies below. 2. If I understood correctly, I see that the CQs are loop-polled in an interrupt context. This may become problematic in stress workload where the CQ simply never drains (imagine some s

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Sagi Grimberg
On 4/16/2014 5:43 PM, Steve Wise wrote: Hmm, But if either FASTREG or LINV failed the QP will go to error state and you *will* get the error wc (with a rain of FLUSH errors). AFAICT it is safe to assume that it succeeded as long as you don't get error completions. But if an unsignaled FASTREG is

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Chuck Lever
Hi Sagi- Thanks for the review! In-line replies below. On Apr 16, 2014, at 9:30 AM, Steve Wise wrote: > On 4/16/2014 7:48 AM, Sagi Grimberg wrote: >> On 4/15/2014 1:23 AM, Chuck Lever wrote: >>> The current CQ handler uses the ib_wc.opcode field to distinguish >>> between event types. However,

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Sagi Grimberg
On 4/16/2014 5:25 PM, Steve Wise wrote: -Original Message- From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il] Sent: Wednesday, April 16, 2014 9:13 AM To: Steve Wise; Chuck Lever; linux-...@vger.kernel.org; linux-rdma@vger.kernel.org Subject: Re: [PATCH 7/8] xprtrdma: Split the

RE: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Steve Wise
> >> Hmm, But if either FASTREG or LINV failed the QP will go to error state > >> and you *will* get the error wc (with a rain of FLUSH errors). > >> AFAICT it is safe to assume that it succeeded as long as you don't get > >> error completions. > > But if an unsignaled FASTREG is posted and silentl

RE: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Steve Wise
> -Original Message- > From: Sagi Grimberg [mailto:sa...@dev.mellanox.co.il] > Sent: Wednesday, April 16, 2014 9:13 AM > To: Steve Wise; Chuck Lever; linux-...@vger.kernel.org; > linux-rdma@vger.kernel.org > Subject: Re: [PATCH 7/8] xprtrdma: Split the completion queue

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Sagi Grimberg
On 4/16/2014 4:30 PM, Steve Wise wrote: On 4/16/2014 7:48 AM, Sagi Grimberg wrote: On 4/15/2014 1:23 AM, Chuck Lever wrote: The current CQ handler uses the ib_wc.opcode field to distinguish between event types. However, the contents of that field are not reliable if the completion status is not

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Steve Wise
On 4/16/2014 7:48 AM, Sagi Grimberg wrote: On 4/15/2014 1:23 AM, Chuck Lever wrote: The current CQ handler uses the ib_wc.opcode field to distinguish between event types. However, the contents of that field are not reliable if the completion status is not IB_WC_SUCCESS. When an error completion

Re: [PATCH 7/8] xprtrdma: Split the completion queue

2014-04-16 Thread Sagi Grimberg
On 4/15/2014 1:23 AM, Chuck Lever wrote: The current CQ handler uses the ib_wc.opcode field to distinguish between event types. However, the contents of that field are not reliable if the completion status is not IB_WC_SUCCESS. When an error completion occurs on a send event, the CQ handler sche

[PATCH 7/8] xprtrdma: Split the completion queue

2014-04-14 Thread Chuck Lever
The current CQ handler uses the ib_wc.opcode field to distinguish between event types. However, the contents of that field are not reliable if the completion status is not IB_WC_SUCCESS. When an error completion occurs on a send event, the CQ handler schedules a tasklet with something that is not