On Wed, 2010-11-17 at 11:57 +0800, Andrew Morton wrote: > On Wed, 17 Nov 2010 11:03:25 +0800 Huang Ying <ying.huang@xxxxxxxxx> wrote: > > > It seems that Steven thinks many architectures without NMI-safe cmpxchg > > have no real NMI too. > > Could be. > > Really, we should nail this down and work out the matrix of > what-works-with-what, and then arrange for the things which _won't_ > work to be either non-Kconfigurable or non-compilable. Yes. I will try to work out a draft matrix for review firstly. > The worst thing we could do would be to advertise some code as > "nmi-safe!!", then to have someone go and use it on that basis, only > for their users to later discover ghastly rare races. Yes. > > In the patch description and comments, it is said that on architectures > > without NMI-safe cmpxchg, gen_pool can not be used in NMI handler > > safely. > > > > Or do you think it is better to use a spin_trylock based fallback if > > NMI-safe cmpxchg is not available? Or require cmpxchg implementation > > uses spin_trylock instead of spin_lock? > > As a first step, a typical thing to do would be to create > CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG, define that in the appropriate > architectures, make ftrace and perf and genpool and anything else > dependent upon that at Kconfig-time. Yes. At least lockless list should depend on that. And we need select between cmpxchg and spin_trylock_irqsave based on that. Hi, Peter, Do you think think irq_work should depend on that? Or we just reimplement irq_work based on lockless list and make irq_work depends on lockless list? > A spin_trylock_irqsave() implementation would do what? Rarely fail the > memory allocation attempt if the trylock failed? I guess that's > acceptable in the context of gen_pool, because memory allocators are > expected to fail, and everyone carefully tests the allocation-failed > error paths (lol). Yes. I will implement the fallback for gen_pool. > But rare failures may not be useful within the context of future > clients of the "lockless" list implementation so I'd say that a safer > approach would be to make the list implementation require > CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG and be done with it. Yes. > So that's all pretty simple so far. However... > > The list implementation is still useful to > non-CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG clients, as long as they aren't > using it from NMI context, yes? > > In which case I suppose we could add a library of lockless_list_foo() > functions which are usable from non-NMI contexts and which are > available to all configs. And on top of that implement a library of > nmi_safe_lockless_list_foo() functions which are just wrappers around > lockless_list_foo(), but which are only available if > CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG=y. > > Which is getting to be a bit of a pain, so you might choose to > disregard everything after "However..." ;) At least as the first step, I prefer to just make lockless list depend on CONFIG_ARCH_HAVE_NMI_SAFE_CMPXCHG. Best Regards, Huang Ying -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html