lkml.org 
[lkml]   [2004]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: netconsole hangs w/ alt-sysrq-t
On Mon, Jun 28, 2004 at 10:48:41AM -0400, Jeff Moyer wrote:
> ==> Regarding Re: netconsole hangs w/ alt-sysrq-t; Matt Mackall <mpm@selenic.com> adds:
>
> [snip]
>
> mpm> Huh. I'm not sure if that covers everything.
>
> mpm> We want to:
>
> mpm> a) push stuff through netpoll_rx() iff rx is enabled b) drop packets
> mpm> when netpoll_rx() says to c) drop packets when we're trapped for
> mpm> debugging/netdump d) drop packets when we manually call dev->poll e)
> mpm> keep as little overhead as possible
>
> mpm> The above breaks (a) when we're not trapped (consider breaking in with
> mpm> debugger) and (b) in any case.
>
> mpm> My current thought is we want to get this down to a single test in the
> mpm> fastpath by replacing netpoll_rx in skb->dev with a flag that is equal
> mpm> to netpoll_trapped() | dev->netpoll_rx, which we manipulate when call
> dev-> poll within netpoll.
>
> mpm> How's that sound?
>
> That sounds fine. So, in the netif_receive_skb function, we'd now have
> something like:
>
> if (skb->dev->netpoll_rx && netpoll_rx(skb)) {
> kfree_skb(skb);
> return NET_RX_DROP;
> }
>
> I removed the test for skb->dev->poll, since this is the NAPI code path, I
> think it should always be present, right?

No, I think we get to this on the non-NAPI route as well. The ->poll
check keeps us from filtering twice.

> And then at the top of the netpoll_rx routine:
>
> if (!(skb->dev->netpoll_rx & NETPOLL_RX_ENABLED))
> goto out;

Let's explicitly return 1.

> The routine, as before, returns the value of trapped. And now,
> netpoll_poll would do this:
>
> if(np->dev->poll && test_bit(__LINK_STATE_RX_SCHED, &np->dev->state)) {
> np->dev->netpoll_rx |= NETPOLL_RX_TRAPPED;
> if (trapped)
> np->dev->poll(np->dev, &budget);
> else {
> trapped = 1;
> np->dev->poll(np->dev, &budget);
> trapped = 0;
> }
> np->dev->netpoll_rx &= ~NETPOLL_RX_TRAPPED;
> }
>
> Is this more in line with what you had in mind?

Yes. Let's make that NETPOLL_RX_DROP to disambiguate the
trapped/dropped-for-polling. And we can do a simple
increment/decrement on trapped itself - but do we still need it here
now that we've shorted out the NAPI path with RX_DROP?

There might be some new atomicity or memory barrier issues here, be
mindful.

> One thing I would consider changing, here, is having
> netpoll_set_trap take a struct netpoll argument. That way, we could
> have netpoll_set_trap manipulate the NETPOLL_RX_TRAPPED bit. This
> would make the trap specific to each interface, but I think that may
> be desirable.

It doesn't really make sense for trapped to be non-global - it means
the machine is in polling-only mode, eg debugger or the like. It's
rather misusing it to kill the NAPI receive side for netpoll_poll.

--
Mathematics is the supreme nostalgia of our time.
-
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/

\
 
 \ /
  Last update: 2005-03-22 14:04    [W:0.074 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site