* [PATCH 1/3] staging/rdma/hfi1: Remove incorrect link credit check
@ 2015-12-18 0:24 Jubin John
[not found] ` <1450398255-10346-1-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Jubin John @ 2015-12-18 0:24 UTC (permalink / raw)
To: gregkh, devel; +Cc: linux-rdma, dledford
From: Dean Luick <dean.luick@intel.com>
Remove an invalid sanity check that compares the local link
credits with the peer link credits. The two have no dependency
on each other.
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Dean Luick <dean.luick@intel.com>
Signed-off-by: Jubin John <jubin.john@intel.com>
---
drivers/staging/rdma/hfi1/chip.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
index dc69159..78a5f08 100644
--- a/drivers/staging/rdma/hfi1/chip.c
+++ b/drivers/staging/rdma/hfi1/chip.c
@@ -7267,8 +7267,7 @@ static int set_buffer_control(struct hfi1_devdata *dd,
new_bc->vl[i].shared = 0;
}
new_total += be16_to_cpu(new_bc->overall_shared_limit);
- if (new_total > (u32)dd->link_credits)
- return -EINVAL;
+
/* fetch the current values */
get_buffer_control(dd, &cur_bc, &cur_total);
--
1.7.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling
[not found] ` <1450398255-10346-1-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2015-12-18 0:24 ` Jubin John
2015-12-18 6:42 ` Dan Carpenter
2015-12-18 0:24 ` [PATCH 3/3] staging/rdma/hfi1: check for ARMED->ACTIVE transition in receive interrupt Jubin John
1 sibling, 1 reply; 7+ messages in thread
From: Jubin John @ 2015-12-18 0:24 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w,
ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
jubin.john-ral2JQCrhuEAvxtiuMwx3w
Fix the spelling of user_credit_return_threshold, it was incorrectly
spelled as user_credit_return_theshold causing two module parameters,
one with typo, to be shown in modinfo
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/staging/rdma/hfi1/init.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/init.c b/drivers/staging/rdma/hfi1/init.c
index 1c8286f..8f13d53 100644
--- a/drivers/staging/rdma/hfi1/init.c
+++ b/drivers/staging/rdma/hfi1/init.c
@@ -113,7 +113,7 @@ MODULE_PARM_DESC(hdrq_entsize, "Size of header queue entries: 2 - 8B, 16 - 64B (
unsigned int user_credit_return_threshold = 33; /* default is 33% */
module_param(user_credit_return_threshold, uint, S_IRUGO);
-MODULE_PARM_DESC(user_credit_return_theshold, "Credit return threshold for user send contexts, return when unreturned credits passes this many blocks (in percent of allocated blocks, 0 is off)");
+MODULE_PARM_DESC(user_credit_return_threshold, "Credit return threshold for user send contexts, return when unreturned credits passes this many blocks (in percent of allocated blocks, 0 is off)");
static inline u64 encode_rcv_header_entry_size(u16);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] staging/rdma/hfi1: check for ARMED->ACTIVE transition in receive interrupt
[not found] ` <1450398255-10346-1-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-12-18 0:24 ` [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling Jubin John
@ 2015-12-18 0:24 ` Jubin John
[not found] ` <1450398255-10346-3-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Jubin John @ 2015-12-18 0:24 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w,
ira.weiny-ral2JQCrhuEAvxtiuMwx3w,
jubin.john-ral2JQCrhuEAvxtiuMwx3w
From: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
The link state will transition from ARMED to ACTIVE when a non-SC15
packet arrives, but the driver might not notice the change. With this
fix, if the slowpath receive interrupt handler sees a non-SC15 packet
while in the ARMED state, we queue work to call linkstate_active_work
from process context to promote it to ACTIVE.
Signed-off-by: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Brendan Cunningham <brendan.cunningham-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/staging/rdma/hfi1/chip.c | 5 ++-
drivers/staging/rdma/hfi1/chip.h | 2 +
drivers/staging/rdma/hfi1/driver.c | 66 ++++++++++++++++++++++++++++++++++++
drivers/staging/rdma/hfi1/hfi.h | 12 ++++++
drivers/staging/rdma/hfi1/init.c | 1 +
5 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
index 78a5f08..f82b848 100644
--- a/drivers/staging/rdma/hfi1/chip.c
+++ b/drivers/staging/rdma/hfi1/chip.c
@@ -4606,7 +4606,7 @@ static inline void clear_recv_intr(struct hfi1_ctxtdata *rcd)
}
/* force the receive interrupt */
-static inline void force_recv_intr(struct hfi1_ctxtdata *rcd)
+void force_recv_intr(struct hfi1_ctxtdata *rcd)
{
write_csr(rcd->dd, CCE_INT_FORCE + (8 * rcd->ireg), rcd->imask);
}
@@ -4705,7 +4705,7 @@ u32 read_physical_state(struct hfi1_devdata *dd)
& DC_DC8051_STS_CUR_STATE_PORT_MASK;
}
-static u32 read_logical_state(struct hfi1_devdata *dd)
+u32 read_logical_state(struct hfi1_devdata *dd)
{
u64 reg;
@@ -6658,6 +6658,7 @@ int set_link_state(struct hfi1_pportdata *ppd, u32 state)
ppd->link_enabled = 1;
}
+ set_all_slowpath(ppd->dd);
ret = set_local_link_attributes(ppd);
if (ret)
break;
diff --git a/drivers/staging/rdma/hfi1/chip.h b/drivers/staging/rdma/hfi1/chip.h
index d74aed8..620cf08 100644
--- a/drivers/staging/rdma/hfi1/chip.h
+++ b/drivers/staging/rdma/hfi1/chip.h
@@ -691,6 +691,8 @@ u64 read_dev_cntr(struct hfi1_devdata *dd, int index, int vl);
u64 write_dev_cntr(struct hfi1_devdata *dd, int index, int vl, u64 data);
u64 read_port_cntr(struct hfi1_pportdata *ppd, int index, int vl);
u64 write_port_cntr(struct hfi1_pportdata *ppd, int index, int vl, u64 data);
+u32 read_logical_state(struct hfi1_devdata *dd);
+void force_recv_intr(struct hfi1_ctxtdata *rcd);
/* Per VL indexes */
enum {
diff --git a/drivers/staging/rdma/hfi1/driver.c b/drivers/staging/rdma/hfi1/driver.c
index 4c52e78..16b1be1 100644
--- a/drivers/staging/rdma/hfi1/driver.c
+++ b/drivers/staging/rdma/hfi1/driver.c
@@ -862,6 +862,15 @@ static inline void set_all_dma_rtail(struct hfi1_devdata *dd)
&handle_receive_interrupt_dma_rtail;
}
+void set_all_slowpath(struct hfi1_devdata *dd)
+{
+ int i;
+
+ for (i = HFI1_CTRL_CTXT + 1; i < dd->first_user_ctxt; i++)
+ dd->rcd[i]->do_interrupt =
+ &handle_receive_interrupt;
+}
+
/*
* handle_receive_interrupt - receive a packet
* @rcd: the context
@@ -929,6 +938,27 @@ int handle_receive_interrupt(struct hfi1_ctxtdata *rcd, int thread)
last = skip_rcv_packet(&packet, thread);
skip_pkt = 0;
} else {
+ /* Auto activate link on non-SC15 packet receive */
+ if (unlikely(rcd->ppd->host_link_state ==
+ HLS_UP_ARMED)) {
+ struct work_struct *lsaw =
+ &rcd->ppd->linkstate_active_work;
+ struct hfi1_message_header *hdr =
+ hfi1_get_msgheader(packet.rcd->dd,
+ packet.rhf_addr);
+ if (hdr2sc(hdr, packet.rhf) != 0xf) {
+ int hwstate = read_logical_state(dd);
+
+ if (hwstate != LSTATE_ACTIVE) {
+ dd_dev_info(dd, "Unexpected link state %d\n",
+ hwstate);
+ } else {
+ queue_work(
+ rcd->ppd->hfi1_wq, lsaw);
+ goto bail;
+ }
+ }
+ }
last = process_rcv_packet(&packet, thread);
}
@@ -984,6 +1014,42 @@ bail:
}
/*
+ * We may discover in the interrupt that the hardware link state has
+ * changed from ARMED to ACTIVE (due to the arrival of a non-SC15 packet),
+ * and we need to update the driver's notion of the link state. We cannot
+ * run set_link_state from interrupt context, so we queue this function on
+ * a workqueue.
+ *
+ * We delay the regular interrupt processing until after the state changes
+ * so that the link will be in the correct state by the time any application
+ * we wake up attempts to send a reply to any message it received.
+ * (Subsequent receive interrupts may possibly force the wakeup before we
+ * update the link state.)
+ *
+ * The rcd is freed in hfi1_free_ctxtdata after hfi1_postinit_cleanup invokes
+ * dd->f_cleanup(dd) to disable the interrupt handler and flush workqueues,
+ * so we're safe from use-after-free of the rcd.
+ */
+void receive_interrupt_work(struct work_struct *work)
+{
+ struct hfi1_pportdata *ppd =
+ container_of(work, struct hfi1_pportdata, linkstate_active_work);
+ struct hfi1_devdata *dd = ppd->dd;
+ int i;
+
+ /* Received non-SC15 packet implies neighbor_normal */
+ ppd->neighbor_normal = 1;
+ set_link_state(ppd, HLS_UP_ACTIVE);
+
+ /*
+ * Interrupt all kernel contexts that could have had an
+ * interrupt during auto activation.
+ */
+ for (i = HFI1_CTRL_CTXT; i < dd->first_user_ctxt; i++)
+ force_recv_intr(dd->rcd[i]);
+}
+
+/*
* Convert a given MTU size to the on-wire MAD packet enumeration.
* Return -1 if the size is invalid.
*/
diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
index 54ed6b3..bfd9723 100644
--- a/drivers/staging/rdma/hfi1/hfi.h
+++ b/drivers/staging/rdma/hfi1/hfi.h
@@ -712,6 +712,7 @@ struct hfi1_pportdata {
u8 remote_link_down_reason;
/* Error events that will cause a port bounce. */
u32 port_error_action;
+ struct work_struct linkstate_active_work;
};
typedef int (*rhf_rcv_function_ptr)(struct hfi1_packet *packet);
@@ -1128,6 +1129,7 @@ void hfi1_free_ctxtdata(struct hfi1_devdata *, struct hfi1_ctxtdata *);
int handle_receive_interrupt(struct hfi1_ctxtdata *, int);
int handle_receive_interrupt_nodma_rtail(struct hfi1_ctxtdata *, int);
int handle_receive_interrupt_dma_rtail(struct hfi1_ctxtdata *, int);
+void set_all_slowpath(struct hfi1_devdata *dd);
/* receive packet handler dispositions */
#define RCV_PKT_OK 0x0 /* keep going */
@@ -1148,6 +1150,16 @@ static inline u32 driver_lstate(struct hfi1_pportdata *ppd)
return ppd->lstate; /* use the cached value */
}
+void receive_interrupt_work(struct work_struct *work);
+
+/* extract service channel from header and rhf */
+static inline int hdr2sc(struct hfi1_message_header *hdr, u64 rhf)
+{
+ return
+ ((be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf) |
+ ((!!(rhf & RHF_DC_INFO_MASK)) << 4);
+}
+
static inline u16 generate_jkey(kuid_t uid)
{
return from_kuid(current_user_ns(), uid) & 0xffff;
diff --git a/drivers/staging/rdma/hfi1/init.c b/drivers/staging/rdma/hfi1/init.c
index 8f13d53..ee206ae 100644
--- a/drivers/staging/rdma/hfi1/init.c
+++ b/drivers/staging/rdma/hfi1/init.c
@@ -498,6 +498,7 @@ void hfi1_init_pportdata(struct pci_dev *pdev, struct hfi1_pportdata *ppd,
INIT_WORK(&ppd->link_downgrade_work, handle_link_downgrade);
INIT_WORK(&ppd->sma_message_work, handle_sma_message);
INIT_WORK(&ppd->link_bounce_work, handle_link_bounce);
+ INIT_WORK(&ppd->linkstate_active_work, receive_interrupt_work);
mutex_init(&ppd->hls_lock);
spin_lock_init(&ppd->sdma_alllock);
spin_lock_init(&ppd->qsfp_info.qsfp_lock);
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] staging/rdma/hfi1: check for ARMED->ACTIVE transition in receive interrupt
[not found] ` <1450398255-10346-3-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2015-12-18 6:31 ` Dan Carpenter
2015-12-23 22:27 ` Jubin John
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2015-12-18 6:31 UTC (permalink / raw)
To: Jubin John, Jim Snow
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA
Possible off by one, but mostly the whitespace makes me itch.
Jim was not on the CC list.
On Thu, Dec 17, 2015 at 07:24:15PM -0500, Jubin John wrote:
> From: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> The link state will transition from ARMED to ACTIVE when a non-SC15
> packet arrives, but the driver might not notice the change. With this
> fix, if the slowpath receive interrupt handler sees a non-SC15 packet
> while in the ARMED state, we queue work to call linkstate_active_work
> from process context to promote it to ACTIVE.
>
> Signed-off-by: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Brendan Cunningham <brendan.cunningham-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/staging/rdma/hfi1/chip.c | 5 ++-
> drivers/staging/rdma/hfi1/chip.h | 2 +
> drivers/staging/rdma/hfi1/driver.c | 66 ++++++++++++++++++++++++++++++++++++
> drivers/staging/rdma/hfi1/hfi.h | 12 ++++++
> drivers/staging/rdma/hfi1/init.c | 1 +
> 5 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
> index 78a5f08..f82b848 100644
> --- a/drivers/staging/rdma/hfi1/chip.c
> +++ b/drivers/staging/rdma/hfi1/chip.c
> @@ -4606,7 +4606,7 @@ static inline void clear_recv_intr(struct hfi1_ctxtdata *rcd)
> }
>
> /* force the receive interrupt */
> -static inline void force_recv_intr(struct hfi1_ctxtdata *rcd)
> +void force_recv_intr(struct hfi1_ctxtdata *rcd)
> {
> write_csr(rcd->dd, CCE_INT_FORCE + (8 * rcd->ireg), rcd->imask);
> }
> @@ -4705,7 +4705,7 @@ u32 read_physical_state(struct hfi1_devdata *dd)
> & DC_DC8051_STS_CUR_STATE_PORT_MASK;
> }
>
> -static u32 read_logical_state(struct hfi1_devdata *dd)
> +u32 read_logical_state(struct hfi1_devdata *dd)
> {
> u64 reg;
>
> @@ -6658,6 +6658,7 @@ int set_link_state(struct hfi1_pportdata *ppd, u32 state)
> ppd->link_enabled = 1;
> }
>
> + set_all_slowpath(ppd->dd);
> ret = set_local_link_attributes(ppd);
> if (ret)
> break;
> diff --git a/drivers/staging/rdma/hfi1/chip.h b/drivers/staging/rdma/hfi1/chip.h
> index d74aed8..620cf08 100644
> --- a/drivers/staging/rdma/hfi1/chip.h
> +++ b/drivers/staging/rdma/hfi1/chip.h
> @@ -691,6 +691,8 @@ u64 read_dev_cntr(struct hfi1_devdata *dd, int index, int vl);
> u64 write_dev_cntr(struct hfi1_devdata *dd, int index, int vl, u64 data);
> u64 read_port_cntr(struct hfi1_pportdata *ppd, int index, int vl);
> u64 write_port_cntr(struct hfi1_pportdata *ppd, int index, int vl, u64 data);
> +u32 read_logical_state(struct hfi1_devdata *dd);
> +void force_recv_intr(struct hfi1_ctxtdata *rcd);
>
> /* Per VL indexes */
> enum {
> diff --git a/drivers/staging/rdma/hfi1/driver.c b/drivers/staging/rdma/hfi1/driver.c
> index 4c52e78..16b1be1 100644
> --- a/drivers/staging/rdma/hfi1/driver.c
> +++ b/drivers/staging/rdma/hfi1/driver.c
> @@ -862,6 +862,15 @@ static inline void set_all_dma_rtail(struct hfi1_devdata *dd)
> &handle_receive_interrupt_dma_rtail;
> }
>
> +void set_all_slowpath(struct hfi1_devdata *dd)
> +{
> + int i;
> +
> + for (i = HFI1_CTRL_CTXT + 1; i < dd->first_user_ctxt; i++)
> + dd->rcd[i]->do_interrupt =
> + &handle_receive_interrupt;
This fits within the 80 character limit.
We start counting from HFI1_CTRL_CTXT + 1 but in receive_interrupt_work()
we start counting from HFI1_CTRL_CTXT. What's the story? It's either a
bug or needs much better documentation.
> +}
> +
> /*
> * handle_receive_interrupt - receive a packet
> * @rcd: the context
> @@ -929,6 +938,27 @@ int handle_receive_interrupt(struct hfi1_ctxtdata *rcd, int thread)
> last = skip_rcv_packet(&packet, thread);
> skip_pkt = 0;
> } else {
> + /* Auto activate link on non-SC15 packet receive */
> + if (unlikely(rcd->ppd->host_link_state ==
> + HLS_UP_ARMED)) {
> + struct work_struct *lsaw =
> + &rcd->ppd->linkstate_active_work;
> + struct hfi1_message_header *hdr =
> + hfi1_get_msgheader(packet.rcd->dd,
> + packet.rhf_addr);
> + if (hdr2sc(hdr, packet.rhf) != 0xf) {
> + int hwstate = read_logical_state(dd);
> +
> + if (hwstate != LSTATE_ACTIVE) {
> + dd_dev_info(dd, "Unexpected link state %d\n",
> + hwstate);
> + } else {
> + queue_work(
> + rcd->ppd->hfi1_wq, lsaw);
> + goto bail;
> + }
> + }
> + }
Can we make this an inline function?
> last = process_rcv_packet(&packet, thread);
> }
>
> @@ -984,6 +1014,42 @@ bail:
> }
>
> /*
> + * We may discover in the interrupt that the hardware link state has
> + * changed from ARMED to ACTIVE (due to the arrival of a non-SC15 packet),
> + * and we need to update the driver's notion of the link state. We cannot
> + * run set_link_state from interrupt context, so we queue this function on
> + * a workqueue.
> + *
> + * We delay the regular interrupt processing until after the state changes
> + * so that the link will be in the correct state by the time any application
> + * we wake up attempts to send a reply to any message it received.
> + * (Subsequent receive interrupts may possibly force the wakeup before we
> + * update the link state.)
> + *
> + * The rcd is freed in hfi1_free_ctxtdata after hfi1_postinit_cleanup invokes
> + * dd->f_cleanup(dd) to disable the interrupt handler and flush workqueues,
> + * so we're safe from use-after-free of the rcd.
> + */
> +void receive_interrupt_work(struct work_struct *work)
> +{
> + struct hfi1_pportdata *ppd =
> + container_of(work, struct hfi1_pportdata, linkstate_active_work);
Normally we would put mostly on the first line.
struct hfi1_pportdata *ppd = container_of(work, struct hfi1_pportdata,
linkstate_active_work);
> + struct hfi1_devdata *dd = ppd->dd;
> + int i;
> +
> + /* Received non-SC15 packet implies neighbor_normal */
> + ppd->neighbor_normal = 1;
> + set_link_state(ppd, HLS_UP_ACTIVE);
> +
> + /*
> + * Interrupt all kernel contexts that could have had an
> + * interrupt during auto activation.
Remove the extra space before "interrupt".
> + */
> + for (i = HFI1_CTRL_CTXT; i < dd->first_user_ctxt; i++)
> + force_recv_intr(dd->rcd[i]);
> +}
> +
> +/*
> * Convert a given MTU size to the on-wire MAD packet enumeration.
> * Return -1 if the size is invalid.
> */
> diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
> index 54ed6b3..bfd9723 100644
> --- a/drivers/staging/rdma/hfi1/hfi.h
> +++ b/drivers/staging/rdma/hfi1/hfi.h
> @@ -712,6 +712,7 @@ struct hfi1_pportdata {
> u8 remote_link_down_reason;
> /* Error events that will cause a port bounce. */
> u32 port_error_action;
> + struct work_struct linkstate_active_work;
> };
>
> typedef int (*rhf_rcv_function_ptr)(struct hfi1_packet *packet);
> @@ -1128,6 +1129,7 @@ void hfi1_free_ctxtdata(struct hfi1_devdata *, struct hfi1_ctxtdata *);
> int handle_receive_interrupt(struct hfi1_ctxtdata *, int);
> int handle_receive_interrupt_nodma_rtail(struct hfi1_ctxtdata *, int);
> int handle_receive_interrupt_dma_rtail(struct hfi1_ctxtdata *, int);
> +void set_all_slowpath(struct hfi1_devdata *dd);
>
> /* receive packet handler dispositions */
> #define RCV_PKT_OK 0x0 /* keep going */
> @@ -1148,6 +1150,16 @@ static inline u32 driver_lstate(struct hfi1_pportdata *ppd)
> return ppd->lstate; /* use the cached value */
> }
>
> +void receive_interrupt_work(struct work_struct *work);
> +
> +/* extract service channel from header and rhf */
> +static inline int hdr2sc(struct hfi1_message_header *hdr, u64 rhf)
> +{
> + return
> + ((be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf) |
> + ((!!(rhf & RHF_DC_INFO_MASK)) << 4);
This should be:
return ((be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf) |
((!!(rhf & RHF_DC_INFO_MASK)) << 4);
> +}
> +
> static inline u16 generate_jkey(kuid_t uid)
> {
> return from_kuid(current_user_ns(), uid) & 0xffff;
> diff --git a/drivers/staging/rdma/hfi1/init.c b/drivers/staging/rdma/hfi1/init.c
> index 8f13d53..ee206ae 100644
> --- a/drivers/staging/rdma/hfi1/init.c
> +++ b/drivers/staging/rdma/hfi1/init.c
> @@ -498,6 +498,7 @@ void hfi1_init_pportdata(struct pci_dev *pdev, struct hfi1_pportdata *ppd,
> INIT_WORK(&ppd->link_downgrade_work, handle_link_downgrade);
> INIT_WORK(&ppd->sma_message_work, handle_sma_message);
> INIT_WORK(&ppd->link_bounce_work, handle_link_bounce);
> + INIT_WORK(&ppd->linkstate_active_work, receive_interrupt_work);
> mutex_init(&ppd->hls_lock);
> spin_lock_init(&ppd->sdma_alllock);
> spin_lock_init(&ppd->qsfp_info.qsfp_lock);
> --
> 1.7.1
>
> _______________________________________________
> devel mailing list
> devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X@public.gmane.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling
2015-12-18 0:24 ` [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling Jubin John
@ 2015-12-18 6:42 ` Dan Carpenter
2015-12-19 0:22 ` Jubin John
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2015-12-18 6:42 UTC (permalink / raw)
To: Jubin John; +Cc: devel, gregkh, dledford, linux-rdma
Nice. Whenever you see a bug like this, you should report it to
kernel-janitors because you know that 10 other people have made the same
mistake.
I will take care of it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling
2015-12-18 6:42 ` Dan Carpenter
@ 2015-12-19 0:22 ` Jubin John
0 siblings, 0 replies; 7+ messages in thread
From: Jubin John @ 2015-12-19 0:22 UTC (permalink / raw)
To: Dan Carpenter; +Cc: devel, gregkh, dledford, linux-rdma
On Fri, Dec 18, 2015 at 09:42:20AM +0300, Dan Carpenter wrote:
> Nice. Whenever you see a bug like this, you should report it to
> kernel-janitors because you know that 10 other people have made the same
> mistake.
Sure, I will do that for similar bugs.
>
> I will take care of it.
Thank you.
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] staging/rdma/hfi1: check for ARMED->ACTIVE transition in receive interrupt
2015-12-18 6:31 ` Dan Carpenter
@ 2015-12-23 22:27 ` Jubin John
0 siblings, 0 replies; 7+ messages in thread
From: Jubin John @ 2015-12-23 22:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jim Snow, Brendan Cunningham,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA
On Fri, Dec 18, 2015 at 09:31:39AM +0300, Dan Carpenter wrote:
> Possible off by one, but mostly the whitespace makes me itch.
>
> Jim was not on the CC list.
>
> On Thu, Dec 17, 2015 at 07:24:15PM -0500, Jubin John wrote:
> > From: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > The link state will transition from ARMED to ACTIVE when a non-SC15
> > packet arrives, but the driver might not notice the change. With this
> > fix, if the slowpath receive interrupt handler sees a non-SC15 packet
> > while in the ARMED state, we queue work to call linkstate_active_work
> > from process context to promote it to ACTIVE.
> >
> > Signed-off-by: Jim Snow <jim.m.snow-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Brendan Cunningham <brendan.cunningham-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Reviewed-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Jubin John <jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/staging/rdma/hfi1/chip.c | 5 ++-
> > drivers/staging/rdma/hfi1/chip.h | 2 +
> > drivers/staging/rdma/hfi1/driver.c | 66 ++++++++++++++++++++++++++++++++++++
> > drivers/staging/rdma/hfi1/hfi.h | 12 ++++++
> > drivers/staging/rdma/hfi1/init.c | 1 +
> > 5 files changed, 84 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/rdma/hfi1/chip.c b/drivers/staging/rdma/hfi1/chip.c
> > index 78a5f08..f82b848 100644
> > --- a/drivers/staging/rdma/hfi1/chip.c
> > +++ b/drivers/staging/rdma/hfi1/chip.c
> > @@ -4606,7 +4606,7 @@ static inline void clear_recv_intr(struct hfi1_ctxtdata *rcd)
> > }
> >
> > /* force the receive interrupt */
> > -static inline void force_recv_intr(struct hfi1_ctxtdata *rcd)
> > +void force_recv_intr(struct hfi1_ctxtdata *rcd)
> > {
> > write_csr(rcd->dd, CCE_INT_FORCE + (8 * rcd->ireg), rcd->imask);
> > }
> > @@ -4705,7 +4705,7 @@ u32 read_physical_state(struct hfi1_devdata *dd)
> > & DC_DC8051_STS_CUR_STATE_PORT_MASK;
> > }
> >
> > -static u32 read_logical_state(struct hfi1_devdata *dd)
> > +u32 read_logical_state(struct hfi1_devdata *dd)
> > {
> > u64 reg;
> >
> > @@ -6658,6 +6658,7 @@ int set_link_state(struct hfi1_pportdata *ppd, u32 state)
> > ppd->link_enabled = 1;
> > }
> >
> > + set_all_slowpath(ppd->dd);
> > ret = set_local_link_attributes(ppd);
> > if (ret)
> > break;
> > diff --git a/drivers/staging/rdma/hfi1/chip.h b/drivers/staging/rdma/hfi1/chip.h
> > index d74aed8..620cf08 100644
> > --- a/drivers/staging/rdma/hfi1/chip.h
> > +++ b/drivers/staging/rdma/hfi1/chip.h
> > @@ -691,6 +691,8 @@ u64 read_dev_cntr(struct hfi1_devdata *dd, int index, int vl);
> > u64 write_dev_cntr(struct hfi1_devdata *dd, int index, int vl, u64 data);
> > u64 read_port_cntr(struct hfi1_pportdata *ppd, int index, int vl);
> > u64 write_port_cntr(struct hfi1_pportdata *ppd, int index, int vl, u64 data);
> > +u32 read_logical_state(struct hfi1_devdata *dd);
> > +void force_recv_intr(struct hfi1_ctxtdata *rcd);
> >
> > /* Per VL indexes */
> > enum {
> > diff --git a/drivers/staging/rdma/hfi1/driver.c b/drivers/staging/rdma/hfi1/driver.c
> > index 4c52e78..16b1be1 100644
> > --- a/drivers/staging/rdma/hfi1/driver.c
> > +++ b/drivers/staging/rdma/hfi1/driver.c
> > @@ -862,6 +862,15 @@ static inline void set_all_dma_rtail(struct hfi1_devdata *dd)
> > &handle_receive_interrupt_dma_rtail;
> > }
> >
> > +void set_all_slowpath(struct hfi1_devdata *dd)
> > +{
> > + int i;
> > +
> > + for (i = HFI1_CTRL_CTXT + 1; i < dd->first_user_ctxt; i++)
> > + dd->rcd[i]->do_interrupt =
> > + &handle_receive_interrupt;
>
> This fits within the 80 character limit.
Will fix in v2.
>
> We start counting from HFI1_CTRL_CTXT + 1 but in receive_interrupt_work()
> we start counting from HFI1_CTRL_CTXT. What's the story? It's either a
> bug or needs much better documentation.
In set_all_slowpath(), we exclude HFI1_CTRL_CTXT because HFI1_CTRL_CTXT
(the SC15, error, and multicast context) must always use the slowpath
handler. We skip HFI1_CTRL_CTXT in set_all_nodma_rtail() and
set_all_dma_rtail() as well.
receive_interrupt_work() starts at HFI1_CTRL_CTXT because we want to
trigger interrupts on all of the receive contexts, including
HFI1_CTRL_CTXT in case any packets were received on any of the contexts
between the interrupt and when the work function is called.
>
> > +}
> > +
> > /*
> > * handle_receive_interrupt - receive a packet
> > * @rcd: the context
> > @@ -929,6 +938,27 @@ int handle_receive_interrupt(struct hfi1_ctxtdata *rcd, int thread)
> > last = skip_rcv_packet(&packet, thread);
> > skip_pkt = 0;
> > } else {
> > + /* Auto activate link on non-SC15 packet receive */
> > + if (unlikely(rcd->ppd->host_link_state ==
> > + HLS_UP_ARMED)) {
> > + struct work_struct *lsaw =
> > + &rcd->ppd->linkstate_active_work;
> > + struct hfi1_message_header *hdr =
> > + hfi1_get_msgheader(packet.rcd->dd,
> > + packet.rhf_addr);
> > + if (hdr2sc(hdr, packet.rhf) != 0xf) {
> > + int hwstate = read_logical_state(dd);
> > +
> > + if (hwstate != LSTATE_ACTIVE) {
> > + dd_dev_info(dd, "Unexpected link state %d\n",
> > + hwstate);
> > + } else {
> > + queue_work(
> > + rcd->ppd->hfi1_wq, lsaw);
> > + goto bail;
> > + }
> > + }
> > + }
>
> Can we make this an inline function?
Yes, will make that change in v2.
>
> > last = process_rcv_packet(&packet, thread);
> > }
> >
> > @@ -984,6 +1014,42 @@ bail:
> > }
> >
> > /*
> > + * We may discover in the interrupt that the hardware link state has
> > + * changed from ARMED to ACTIVE (due to the arrival of a non-SC15 packet),
> > + * and we need to update the driver's notion of the link state. We cannot
> > + * run set_link_state from interrupt context, so we queue this function on
> > + * a workqueue.
> > + *
> > + * We delay the regular interrupt processing until after the state changes
> > + * so that the link will be in the correct state by the time any application
> > + * we wake up attempts to send a reply to any message it received.
> > + * (Subsequent receive interrupts may possibly force the wakeup before we
> > + * update the link state.)
> > + *
> > + * The rcd is freed in hfi1_free_ctxtdata after hfi1_postinit_cleanup invokes
> > + * dd->f_cleanup(dd) to disable the interrupt handler and flush workqueues,
> > + * so we're safe from use-after-free of the rcd.
> > + */
> > +void receive_interrupt_work(struct work_struct *work)
> > +{
> > + struct hfi1_pportdata *ppd =
> > + container_of(work, struct hfi1_pportdata, linkstate_active_work);
>
> Normally we would put mostly on the first line.
>
> struct hfi1_pportdata *ppd = container_of(work, struct hfi1_pportdata,
> linkstate_active_work);
Fixed in v2.
>
> > + struct hfi1_devdata *dd = ppd->dd;
> > + int i;
> > +
> > + /* Received non-SC15 packet implies neighbor_normal */
> > + ppd->neighbor_normal = 1;
> > + set_link_state(ppd, HLS_UP_ACTIVE);
> > +
> > + /*
> > + * Interrupt all kernel contexts that could have had an
> > + * interrupt during auto activation.
>
> Remove the extra space before "interrupt".
Fixed in v2.
>
> > + */
> > + for (i = HFI1_CTRL_CTXT; i < dd->first_user_ctxt; i++)
> > + force_recv_intr(dd->rcd[i]);
> > +}
> > +
> > +/*
> > * Convert a given MTU size to the on-wire MAD packet enumeration.
> > * Return -1 if the size is invalid.
> > */
> > diff --git a/drivers/staging/rdma/hfi1/hfi.h b/drivers/staging/rdma/hfi1/hfi.h
> > index 54ed6b3..bfd9723 100644
> > --- a/drivers/staging/rdma/hfi1/hfi.h
> > +++ b/drivers/staging/rdma/hfi1/hfi.h
> > @@ -712,6 +712,7 @@ struct hfi1_pportdata {
> > u8 remote_link_down_reason;
> > /* Error events that will cause a port bounce. */
> > u32 port_error_action;
> > + struct work_struct linkstate_active_work;
> > };
> >
> > typedef int (*rhf_rcv_function_ptr)(struct hfi1_packet *packet);
> > @@ -1128,6 +1129,7 @@ void hfi1_free_ctxtdata(struct hfi1_devdata *, struct hfi1_ctxtdata *);
> > int handle_receive_interrupt(struct hfi1_ctxtdata *, int);
> > int handle_receive_interrupt_nodma_rtail(struct hfi1_ctxtdata *, int);
> > int handle_receive_interrupt_dma_rtail(struct hfi1_ctxtdata *, int);
> > +void set_all_slowpath(struct hfi1_devdata *dd);
> >
> > /* receive packet handler dispositions */
> > #define RCV_PKT_OK 0x0 /* keep going */
> > @@ -1148,6 +1150,16 @@ static inline u32 driver_lstate(struct hfi1_pportdata *ppd)
> > return ppd->lstate; /* use the cached value */
> > }
> >
> > +void receive_interrupt_work(struct work_struct *work);
> > +
> > +/* extract service channel from header and rhf */
> > +static inline int hdr2sc(struct hfi1_message_header *hdr, u64 rhf)
> > +{
> > + return
> > + ((be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf) |
> > + ((!!(rhf & RHF_DC_INFO_MASK)) << 4);
>
> This should be:
>
> return ((be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf) |
> ((!!(rhf & RHF_DC_INFO_MASK)) << 4);
Fixed in v2.
>
> > +}
> > +
> > static inline u16 generate_jkey(kuid_t uid)
> > {
> > return from_kuid(current_user_ns(), uid) & 0xffff;
> > diff --git a/drivers/staging/rdma/hfi1/init.c b/drivers/staging/rdma/hfi1/init.c
> > index 8f13d53..ee206ae 100644
> > --- a/drivers/staging/rdma/hfi1/init.c
> > +++ b/drivers/staging/rdma/hfi1/init.c
> > @@ -498,6 +498,7 @@ void hfi1_init_pportdata(struct pci_dev *pdev, struct hfi1_pportdata *ppd,
> > INIT_WORK(&ppd->link_downgrade_work, handle_link_downgrade);
> > INIT_WORK(&ppd->sma_message_work, handle_sma_message);
> > INIT_WORK(&ppd->link_bounce_work, handle_link_bounce);
> > + INIT_WORK(&ppd->linkstate_active_work, receive_interrupt_work);
> > mutex_init(&ppd->hls_lock);
> > spin_lock_init(&ppd->sdma_alllock);
> > spin_lock_init(&ppd->qsfp_info.qsfp_lock);
> > --
> > 1.7.1
> >
> > _______________________________________________
> > devel mailing list
> > devel-tBiZLqfeLfOHmIFyCCdPziST3g8Odh+X@public.gmane.org
> > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-23 22:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-18 0:24 [PATCH 1/3] staging/rdma/hfi1: Remove incorrect link credit check Jubin John
[not found] ` <1450398255-10346-1-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-12-18 0:24 ` [PATCH 2/3] staging/rdma/hfi1: Fix module parameter spelling Jubin John
2015-12-18 6:42 ` Dan Carpenter
2015-12-19 0:22 ` Jubin John
2015-12-18 0:24 ` [PATCH 3/3] staging/rdma/hfi1: check for ARMED->ACTIVE transition in receive interrupt Jubin John
[not found] ` <1450398255-10346-3-git-send-email-jubin.john-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-12-18 6:31 ` Dan Carpenter
2015-12-23 22:27 ` Jubin John
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.