On Wed, Mar 10, 2021 at 02:10:12PM -0800, Mike Kravetz wrote: > On 3/10/21 1:49 PM, Paul E. McKenney wrote: > > On Wed, Mar 10, 2021 at 10:11:22PM +0100, Michal Hocko wrote: > >> On Wed 10-03-21 10:56:08, Mike Kravetz wrote: > >>> On 3/10/21 7:19 AM, Michal Hocko wrote: > >>>> On Mon 08-03-21 18:28:02, Muchun Song wrote: > >>>> [...] > >>>>> @@ -1447,7 +1486,7 @@ void free_huge_page(struct page *page) > >>>>> /* > >>>>> * Defer freeing if in non-task context to avoid hugetlb_lock deadlock. > >>>>> */ > >>>>> - if (!in_task()) { > >>>>> + if (in_atomic()) { > >>>> > >>>> As I've said elsewhere in_atomic doesn't work for CONFIG_PREEMPT_COUNT=n. > >>>> We need this change for other reasons and so it would be better to pull > >>>> it out into a separate patch which also makes HUGETLB depend on > >>>> PREEMPT_COUNT. > >>> > >>> Yes, the issue of calling put_page for hugetlb pages from any context > >>> still needs work. IMO, that is outside the scope of this series. We > >>> already have code in this path which blocks/sleeps. > >>> > >>> Making HUGETLB depend on PREEMPT_COUNT is too restrictive. IIUC, > >>> PREEMPT_COUNT will only be enabled if we enable: > >>> PREEMPT "Preemptible Kernel (Low-Latency Desktop)" > >>> PREEMPT_RT "Fully Preemptible Kernel (Real-Time)" > >>> or, other 'debug' options. These are not enabled in 'more common' > >>> kernels. Of course, we do not want to disable HUGETLB in common > >>> configurations. > >> > >> I haven't tried that but PREEMPT_COUNT should be selectable even without > >> any change to the preemption model (e.g. !PREEMPT). > > > > It works reliably for me, for example as in the diff below. So, > > as Michal says, you should be able to add "select PREEMPT_COUNT" to > > whatever Kconfig option you need to. > > > > Thanks Paul. > > I may have been misreading Michal's suggestion of "make HUGETLB depend on > PREEMPT_COUNT". We could "select PREEMPT_COUNT" if HUGETLB is enabled. > However, since HUGETLB is enabled in most configs, then this would > result in PREEMPT_COUNT also being enabled in most configs. I honestly > do not know how much this will cost us? I assume that if it was free or > really cheap it would already be always on? There are a -lot- of configs out there, so are you sure that HUGETLB is really enabled in most of them? ;-) More seriously, I was going by earlier emails in this and related threads plus Michal's "PREEMPT_COUNT should be selectable". But there are other situations that would like PREEMPT_COUNT. And to your point, some who would rather PREEMPT_COUNT not be universally enabled. I haven't seen any performance or kernel-size numbers from any of them, however. Thanx, Paul > -- > Mike Kravetz > > > Thanx, Paul > > > > diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig > > index 3128b7c..7d9f989 100644 > > --- a/kernel/rcu/Kconfig > > +++ b/kernel/rcu/Kconfig > > @@ -8,6 +8,7 @@ menu "RCU Subsystem" > > config TREE_RCU > > bool > > default y if SMP > > + select PREEMPT_COUNT > > help > > This option selects the RCU implementation that is > > designed for very large SMP system with hundreds or > >