Messages in this thread | | | Date | Fri, 12 May 2006 01:53:44 +0200 | From | Brice Goglin <> | Subject | Re: [PATCH 4/6] myri10ge - First half of the driver |
| |
Francois Romieu wrote: > >> + spin_lock(&mgp->cmd_lock); >> + response->result = 0xffffffff; >> + mb(); >> + myri10ge_pio_copy((void __iomem *) cmd_addr, buf, sizeof (*buf)); >> + >> + /* wait up to 2 seconds */ >> > > You must not hold a spinlock for up to 2 seconds. >
We are working on reducing the delay to about 15ms. It only occurs when the driver is loaded or the link brought up.
>> + for (sleep_total = 0; sleep_total < (2 * 1000); sleep_total += 10) { >> + mb(); >> + if (response->result != 0xffffffff) { >> + if (response->result == 0) { >> + data->data0 = ntohl(response->data); >> + spin_unlock(&mgp->cmd_lock); >> + return 0; >> + } else { >> + dev_err(&mgp->pdev->dev, >> + "command %d failed, result = %d\n", >> + cmd, ntohl(response->result)); >> + spin_unlock(&mgp->cmd_lock); >> + return -ENXIO; >> > > Return in a middle of a spinlock-intensive function. :o( >
What do you mean ?
> >> +{ >> + struct sk_buff *skb; >> + unsigned long data, roundup; >> + >> + skb = dev_alloc_skb(bytes + 4096 + MYRI10GE_MCP_ETHER_PAD); >> + if (skb == NULL) >> + return NULL; >> > > Imho you will want to work directly with pages shortly. >
We had thought about doing this, but were a little nervous since we did not know of any other drivers that worked directly with pages. If this is an official direction to work directly with pages, we will. But the existing approach is well tested through our beta cycle, and we would prefer to leave it as is and update to a pages based approach in the future.
Thanks a lot for all the comments.
Brice
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |