Hi, Thank you for the review, Jason. Comments inline.
----- Original Message ----- > From: "Jason Wessel" <[email protected]> > To: "Andrei Warkentin" <[email protected]> > Cc: [email protected], [email protected], "Andrei Warkentin" > <[email protected]>, > [email protected], "Matt Mackall" <[email protected]> > Sent: Monday, February 27, 2012 6:17:12 PM > Subject: Re: [PATCHv3 1/3] NETPOLL: Extend rx_hook support. > > > I am just now starting to look how this patch set compares to kgdboe. > For the kgdboe the patch is a bit different. The kgdboe opted to > just pass the skb so as to cut down on the number of arguments to > the function call. > > From the kgdboe patch: > > - void (*rx_hook)(struct netpoll *, int, char *, int); > + void (*rx_hook)(struct netpoll *, int, char *, int, struct > sk_buff *); > > Interesting, I thought about passing the skb, but decided I didn't want to copy and paste the skb parsing code, especially given that it's always UDP anyway. I still have reservations about passing the physical address, but I don't think anyone tried to use netpoll or a non-ethernet device anyway. > > > > +void netpoll_poll_dev(struct net_device *dev); > > void netpoll_send_udp(struct netpoll *np, const char *msg, int > > > > -static void netpoll_poll_dev(struct net_device *dev) > > +void netpoll_poll_dev(struct net_device *dev) > > > This is interesting and I will have to look into this further... A > large part of the reason kgdboe never went anywhere was all around > the locking problems the ability to safely use the network hardware > and restore the state when it was done. It appears you made this > change so as to make a lockless call directly instead of going > through netpoll_poll(). I am not entirely sure you could safely do > this. > > In kgdboe we always had: > > +static int eth_get_char(void) > +{ > + int chr; > + > + while (atomic_read(&in_count) == 0) > + netpoll_poll(&np); > > > If it is the case that you really can safely call the > netpoll_poll_dev() without the locks then the horrible sync irq > state etc... could go away in kgdboe, and then it would be worth > considering digging up all the ethernet polling errata fixes that > live of out the mainline and perhaps submit some for review. > I didn't look deeply at kgdboe (probably should have...). Anyway, netpoll_poll doesn't seem to exist. netpoll_poll_dev is called from netpoll_send_skb_on_dev and the only contract I see is running with the interrupts disabled - something that is satisfied by running in the context of KDB. This is slight OT, but...are WiFi drivers sufficiently similar that netpoll "just works?" Thanks again. A ------------------------------------------------------------------------------ Try before you buy = See our experts in action! The most comprehensive online learning library for Microsoft developers is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3, Metro Style Apps, more. Free future releases when you subscribe now! http://p.sf.net/sfu/learndevnow-dev2 _______________________________________________ Kgdb-bugreport mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
