Andrew Morton wrote:
> 
> Jim Keniston <jkenisto@us.ibm.com> wrote:
> >
> > +int kernel_error_event_iov(const struct iovec *iov, unsigned int nseg,
> > +     u32 groups)
> > +{
> > ...
> > +
> > +     return netlink_broadcast(kerror_nl, skb, 0, ~0, GFP_ATOMIC);
> 
> This appears to be deadlocky when called from interrupt handlers.
> 
> netlink_broadcast() does read_lock(&nl_table_lock).  But nl_table_lock is
> not an irq-safe lock.
> 
> Possibly netlink_broadcast() can be made callable from hardirq context, but
> it looks to be non trivial.  The various error and delivery handlers need
> to be reviewed,

Yes indeed.  I believe this issue is resolved by detecting that we're in an
interrupt handler and delaying the call to netlink_broadcast() via a tasklet.
See the enclosed patch to the previously posted rev.  I'll update the full patch at
http://prdownloads.sourceforge.net/evlog/kerror-2.5.75.patch?download

An issue remains: what, if anything, to tell the caller if the delayed
netlink_broadcast() fails.  See below for further thoughts.

> the kfree_skb() calls should be thought about, etc.

I've thought about them. :-)  Given the aforementioned solution, I don't think
kfree_skb() is an issue, because it's called as needed by netlink_broadcast().
If I'm missing something here, feel free to clarify.

Thanks.
Jim

WHAT IF THE DELAYED netlink_broadcast() CALL FAILS?
1. Can we detect from IRQ context that nobody is listening, and thereby
return -ESRCH to the caller?  No, to do that would require perusing the
nl_table[NETLINK_KERROR] list.  We can't do that for the same reason we
can't call netlink_broadcast().  So kernel_error_event_iov() now returns
-EINPROGRESS if it had to delay the netlink_broadcast() call.

2. Could the tasklet report netlink_broadcast() failures back to the
higher-level code?  Yes, we could implement a per-group callback to handle
that.  Current thinking is that it's overkill.  But it would resolve
the next issue...

3. Given the above, what should the evlog.c caller do when
kernel_error_event_iov() returns -EINPROGRESS?
a. Nothing.  Figure the packet will probably get logged.
b. Just to be safe, report it via printk, the same way we report dropped
packets.
We currently do (a).  (b) would mean that every event logged from IRQ
context would be cc-ed to printk.
-----