Michael Buesch wrote:
On Wednesday 01 August 2007 10:31:17 Michael Chan wrote:
+static irqreturn_t bnx2x_msix_sp_int(int irq, void *dev_instance)
+{
+ struct net_device *dev = dev_instance;
You need to check if dev==NULL and bail out.
Another driver sharing the IRQ with this might choose to pass the dev
pointer as NULL.
NAK that advice: It is pointless having such a check in the hottest of
driver hot paths, since a large majority of drivers do not have such a
check.
It is better to fix the extremely rare oddball that passes NULL to
request_irq(), than to update all drivers to be slower due to the oddballs.
+ struct bnx2x *bp = netdev_priv(dev);
No check if the device actually _did_ generate the IRQ? Sharing...
Not for MSI
+static irqreturn_t bnx2x_msix_fp_int(int irq, void *fp_cookie)
+{
+
+ struct bnx2x_fastpath *fp = fp_cookie;
Check if fp==NULL
NAK
+ struct bnx2x *bp = fp->bp;
+ struct net_device *dev = bp->dev;
No share protection either?
MSI
+static irqreturn_t bnx2x_interrupt(int irq, void *dev_instance)
+{
+ struct net_device *dev = dev_instance;
Check if dev==NULL
NAK
+ struct bnx2x *bp = netdev_priv(dev);
+ u16 status = bnx2x_ack_int(bp);
+
+ if (unlikely(status == 0)) {
That's not unlikely.
in this case, agreed
the other comments seem fairly sane, and indeed should be considered for
the existing drivers as well.
Jeff
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
