irq_work
in [Kernel]
|
From: Frederic Weisbecker on 25 Jun 2010 05:50 2010/6/25 Huang Ying <ying.huang(a)intel.com>: > On Fri, 2010-06-25 at 17:23 +0800, Frederic Weisbecker wrote: >> 2010/6/25 Huang Ying <ying.huang(a)intel.com>: >> > On Fri, 2010-06-25 at 15:48 +0800, Peter Zijlstra wrote: >> >> On Fri, 2010-06-25 at 10:12 +0800, Huang Ying wrote: >> >> > >> >> > It is better to add "void *data" field in this struct to allow same >> >> > function can be used for multiple struct irq_work. >> >> >> >> No, simply do: >> >> >> >> struct my_foo { >> >> � struct irq_work work; >> >> � /* my extra data */ >> >> } >> >> >> >> void my_func(struct irq_work *work) >> >> { >> >> � struct my_foo *foo = container_of(work, struct my_foo, work); >> >> >> >> � /* tada! */ >> >> } >> > >> > Yes. This works too. But Adding "void *data" field is helpful if you do >> > not embed struct irq_work into another struct. >> >> >> That's what makes most sense. If you use work->data to put foo, then >> you can also do the opposite. Now the best is to pick the choice that >> gives you a real type and a typechecking, and not an error-prone and >> obfuscated void * >> >> This is the way things are made in the kernel. struct work_struct, struct list, >> struct rcu_head, etc... are all embedded into a container, so that we can >> use container_of. > > container_of has no full type checking too. You're right. There is nothing that guarantees B is contained into A, I mean the code is supposed to provide this guarantee, but not the type. That said it's much proper than playing with a void *data, beside the fact kernel developers will quickly understand what you do if you play with such scheme as they are used to it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 25 Jun 2010 06:40 On Thu, 2010-06-24 at 14:38 +0200, Andi Kleen wrote: > The sleepable > soft irq would have avoided that (that's not a show stopper) I'm still not convinced sleepable softirq is a workable thing. Softirqs: A) are non-preemptible B) are per-cpu because of A C) can be ran from ksoftirqd context D) generic kernel infrastructure with identical semantics on all archs If you were to make something like a sleepable softirq, you'd loose A (per definition), B (sleepable implies migratable to cpus_allowed) and possibly D (unless you want to touch all architectures). Now from your 'requirements': > I have one case that needs to sleep (but only when interrupting user code) > TIF works for user space, but it's a bit ugly because it requires adding > more data to the task_struct because CPUs can change. Which I read as: 1) needs to run in the task context of the task that got 'interrupted' 2) needs to stay on the cpu it got interrupted on. So C is out of the window too, at which point there's nothing resembling softirqs left. To boot, x86_64 runs softirqs from the hardirq stack: /* Call softirq on interrupt stack. Interrupts are off. */ ENTRY(call_softirq) CFI_STARTPROC push %rbp CFI_ADJUST_CFA_OFFSET 8 CFI_REL_OFFSET rbp,0 mov %rsp,%rbp CFI_DEF_CFA_REGISTER rbp incl PER_CPU_VAR(irq_count) cmove PER_CPU_VAR(irq_stack_ptr),%rsp push %rbp # backlink for old unwinder call __do_softirq leaveq CFI_DEF_CFA_REGISTER rsp CFI_ADJUST_CFA_OFFSET -8 decl PER_CPU_VAR(irq_count) ret CFI_ENDPROC END(call_softirq) Also, -rt has something that could be considered sleepable softirqs, although we call them preemptible softirqs. It runs all softirqs from cpu bound kthreads, which again doesn't match your requirements. So no, I don't think your idea of sleepable softirqs is sound. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: huang ying on 25 Jun 2010 08:00
On Fri, Jun 25, 2010 at 5:30 PM, Peter Zijlstra <peterz(a)infradead.org> wrote: >> > You can always miss an NMI since it can always happen before the >> > callback gets done, and allowing another enqueue before the callback is >> > complete is asking for trouble. >> >> If we move entry->next = NULL before entry->func(entry), we will not >> miss the NMI. Can you show how to miss it in this way? > > <NMI> > ... > irq_work_queue(&my_work, func); > ... > <EOI> > <IPI> > irq_work_run() > > <NMI> > irq_work_queue(&my_work, func); <FAIL> > <EOI> > > my_func.next = NULL; entry->func() should follows here. You can collect all information (maybe some data in a ring buffer) from NMI handler in entry->func(). But if you place entry->NULL after entry->func(), you will really lose a NMI notification and the information from NMI handler. > <EOI> > Really not that hard. Now imagine wrapping irq_work in some state and > you reusing the state while the function is still running.. So I suggest to use another flag to signify the function is running to distinguish. Best Regards, Huang Ying -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |