Hi Jarek,
On Thursday 12 July 2007 14:59, Jarek Poplawski wrote:
> > +#ifdef CONFIG_NETPOLL
> > + /* Prevent race with netpoll - yes, this is a kludge.
> > + * But at least it doesn't penalize the non-netpoll
> > + * code path. */
>
> Alas, this can penalize those who have it enabled (e.g. by distro),
> but don't use.
Well, the test_bit is actually cheap; it's not atomic, and has no memory
ordering requirements by all I know. The costly thing is set_bit/clear_bit
in poll_napi; and you only ever get there when you *use* netpoll.
> And it looks like _netif_rx_complete should be a better place,
> at least considering such cards as: 8139too, skge, sungem and
> maybe more (according to 2.6.22).
Why?
> > + set_bit(__LINK_STATE_POLL_LIST_FROZEN, &np->dev->state);
> > npinfo->rx_flags |= NETPOLL_RX_DROP;
>
> I wonder, why this flag cannot be used for this check?
I tried, but it made the patch rather icky. netpoll_info is defined
in netpoll.h, which includes netdevice.h. So you cannot inline the
check, and have to use an out-of-line function instead, along the
lines of
extern int am_i_being_called_by_poll_napi(struct net_device *);
netif_rx_complete(struct net_device *dev)
{
#ifdef CONFIG_NETPOLL
if (unlikely(dev->npinfo && am_i_being_called_by_poll_napi(dev))
return;
#endif
...
}
If you don't mind that, yes - this flag (or better, a newly introduced
NETPOLL_RX_NAPI) may work as well.
One thing I was a little worried about was whether dev->npinfo can
go away all of a sudden. It's really just protected by an rcu_readlock...
> BTW, I'd be very glad if somebody could hint me what is the main
> reason for such troublesome function as poll_napi: if it's about
> performance isn't this enough to increase budget for netpoll in
> net_rx_action?
I think one reason is that you want to get the kernel oops out even
when the machine is so hosed that it doesn't even service softirqs
anymore.
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
[EMAIL PROTECTED] | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
-
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
