Re: [PATCH -mm 5/9] netconsole: Introduce dev_status member

On Wed, 4 Jul 2007 08:56:42 -0500
Matt Mackall [off-list ref] wrote:

On Wed, Jul 04, 2007 at 04:38:04PM +0530, Satyam Sharma wrote:

quoted

From: Satyam Sharma <redacted>

[5/9] netconsole: Introduce dev_status member

Introduce a new member in netconsole_target that tracks the status (up or
down) of the underlying interface network device that the specific logging
target netpoll is attached to.

We then join this up with the just-introduced net_device notifier, and
introduce NETDEV_UP and NETDEV_DOWN notifications. By disabling the target
when the corresponding local interface is down, we save the overhead of
unnecessarily disabling interrupts and calling into the netpoll stack in
console->write().

Yuck. 

quoted

+/*
+ * Why no net_dev_is_up() in netdevice.h? The kernel could lose a lot of
+ * weight if only netdevice.h had the good sense to export such a function.
+ * Oh well ...
+ */
+static inline int net_dev_is_up(struct net_device *net_dev)
+{
+	return ((net_dev->flags & IFF_UP) == IFF_UP);
+}

Why editorialize? Why not just add this to netdevice.h?

quoted

+	if (nt->dev_status) {

Why not simply call net_dev_is_up?

The flags field values are really BSD legacy-ish stuff and should not be
used internally. IFF_UP etc, do not have the necessary atomic properties.

Use netif_running() instead.