All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: Paul Gortmaker <paul.gortmaker@windriver.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Hillf Danton <hillf.zj@alibaba-inc.com>,
	"'Andrew Morton'" <akpm@linux-foundation.org>,
	"'Nadia Yvette Chambers'" <nyc@holomorphy.com>,
	"'Alexander Viro'" <viro@zeniv.linux.org.uk>,
	"'Naoya Horiguchi'" <n-horiguchi@ah.jp.nec.com>,
	"'David Rientjes'" <rientjes@google.com>,
	"'Davidlohr Bueso'" <dave@stgolabs.net>,
	"'Linux Kernel Mailing List'" <linux-kernel@vger.kernel.org>
Subject: Re: Regression: 4.5-rc1 (bisect: hugetlb: make mm and fs code explicitly non-modular vs CONFIG_TIMER_STATS)
Date: Thu, 28 Jan 2016 14:18:05 -0800	[thread overview]
Message-ID: <56AA939D.6080104@oracle.com> (raw)
In-Reply-To: <56AA2E44.4070709@oracle.com>

On 01/28/2016 07:05 AM, Mike Kravetz wrote:
> On 01/28/2016 06:37 AM, Paul Gortmaker wrote:
>> [Re: Regression: 4.5-rc1 (bisect: hugetlb: make mm and fs code explicitly non-modular vs CONFIG_TIMER_STATS)] On 28/01/2016 (Thu 10:48) Christian Borntraeger wrote:
>>
>>> On 01/28/2016 10:40 AM, Hillf Danton wrote:
>>>>>
>>>>> Paul,
>>>>>
>>>>> the commit 3e89e1c5ea842 ("hugetlb: make mm and fs code explicitly non-modular")
>>>>> triggers belows warning/oops, if CONFIG_TIMER_STATS is set.
>>>>>
>>>>> Looking at the patch the only "real" change is the init_call,
>>>>> and indeed
>>>>> --- a/mm/hugetlb.c
>>>>> +++ b/mm/hugetlb.c
>>>>> @@ -2653,7 +2653,7 @@ static int __init hugetlb_init(void)
>>>>>                 mutex_init(&hugetlb_fault_mutex_table[i]);
>>>>>         return 0;
>>>>>  }
>>>>> -subsys_initcall(hugetlb_init);
>>>>> +device_initcall(hugetlb_init);
>>>>>
>>>>>  /* Should be called on processing a hugepagesz=... option */
>>>>>  void __init hugetlb_add_hstate(unsigned int order)
>>>>>
>>>>> makes the problem go away.
>>>>
>>>> Helps more if a patch is delivered.
>>>
>>> The problem is that the original change was intentional. So I do not not
>>> what the right fix is.
>>
>> Thanks for the report ; let me see if I can work out what TIMER_STATS
>> is doing to cause this sometime today.
>>
> 
> Hmmm?  CONFIG_TIMER_STATS is set in my config and I am not seeing the
> issue.  Not sure, but it looks like Christian is building/running on
> s390. This 'might' be a contributing factor.

I do not see how CONFIG_TIMER_STATS contributes to this issue.  However,
on s390 numa nodes are initialized at device_initcall in the appropriately
named routine numa_init_late().  hugetlb_init must be done after numa
initialization.  So, I suggest we just move the hugetlb initialization
back to device_initcall.  What do you think Paul?  Patch below.

Is there a way to add checks for this type of thing in the code?  I'm
guessing not.

-- 
Mike Kravetz

hugetlb: move hugetlb_init and init_hugetlbfs_fs to device_initcall level

hugetlb_init() must be called after numa initialization for all
architectures.  Currently, numa initialization happens as late as
device_initcall.  Therefore, move hugetlb_init to device_initcall
level.  init_hugetlbfs_fs() depends on hugetlb_init(), so move it
to device_initcall as well.

Fixes: 3e89e1c5ea ("hugetlb: make mm and fs code explicitly non-modular")
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 fs/hugetlbfs/inode.c | 3 ++-
 mm/hugetlb.c         | 6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 540ddc9..d27d3f6 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1363,4 +1363,5 @@ static int __init init_hugetlbfs_fs(void)
  out2:
 	return error;
 }
-fs_initcall(init_hugetlbfs_fs)
+/* Must happen after hugetlb_init() */
+device_initcall(init_hugetlbfs_fs)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 12908dc..a4c0015 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2653,7 +2653,11 @@ static int __init hugetlb_init(void)
 		mutex_init(&hugetlb_fault_mutex_table[i]);
 	return 0;
 }
-subsys_initcall(hugetlb_init);
+/*
+ * hugetlb_init must be called after numa initialization for all
architectures.
+ * Currently, this is as late as device_initcall().
+ */
+device_initcall(hugetlb_init);

 /* Should be called on processing a hugepagesz=... option */
 void __init hugetlb_add_hstate(unsigned int order)
-- 
2.4.3

  reply	other threads:[~2016-01-28 22:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-28  9:16 Regression: 4.5-rc1 (bisect: hugetlb: make mm and fs code explicitly non-modular vs CONFIG_TIMER_STATS) Christian Borntraeger
2016-01-28  9:40 ` Hillf Danton
2016-01-28  9:48   ` Christian Borntraeger
2016-01-28 14:37     ` Paul Gortmaker
2016-01-28 15:05       ` Mike Kravetz
2016-01-28 22:18         ` Mike Kravetz [this message]
2016-01-28 22:59           ` Paul Gortmaker
2016-01-29  0:27             ` Mike Kravetz
2016-01-29  8:23               ` Christian Borntraeger
2016-01-29 14:24               ` Paul Gortmaker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56AA939D.6080104@oracle.com \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=borntraeger@de.ibm.com \
    --cc=dave@stgolabs.net \
    --cc=hillf.zj@alibaba-inc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=nyc@holomorphy.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=rientjes@google.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.