Messages in this thread | | | Date | Fri, 5 Aug 2011 14:26:12 -0700 | Subject | Re: [PATCH v4 2/2] Output stall traces in /proc | From | ZAK Magnus <> |
| |
On Fri, Aug 5, 2011 at 11:43 AM, Don Zickus <dzickus@redhat.com> wrote: > I missed that you defined that as a pointer to a spinlock and assigned it > later. I see what you are doing now, but I am not a fan of it because you > are now using the same spinlock in both the NMI context and the userspace > context. This can cause deadlocks if something got screwed up in the > seq_printf functions or produced a very large amount of data. Normally > you don't want to do that. > > What others have done like perf and the APEI error handling is use > something called irq_work_queue(??). Basically you would capture the > tracae in the NMI context, put it on an irq_work_queue and in the > interrupt context save it to your global trace variable. Then you could > put spin_lock_irqsave inside the proc sys function and the work queue > function and not have any potential deadlocks. Work queue? Okay. The worker thread still needs a lock in order to share the intermediate buffer with the NMI context, though. Any chance of something screwing up in the middle of copying that structure, causing a stall and deadlocking with the NMI?
Or maybe the intermediate buffer should be dynamically allocated. That would work without a lock, although it seems slightly inefficient.
Regarding the lock between the work queue thread and the system call, maybe that should become a mutex instead, since it's all outside of interrupt context at that point?
> The softstall case should be ok though. Why's that? The soft stall traces are not written in a NMI context but just in a regular interrupt context, right? Doesn't that pose similar problems?
These are weird rare corner cases anyway, right? Maybe the simplest thing could be to let the interrupts only try_lock(), so they might sometimes fail to record a stall, but it would be a pretty big coincidence. -- 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/
| |