Re: [PATCH v18 4/9] mm: hugetlb: alloc the vmemmap pages associated with each HugeTLB page

[Mike Kravetz <mike.kravetz@oracle.com>]

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?

-- 
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
>