On 2022/6/9 13:24, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Thu, Jun 09, 2022 at 01:00:28PM +0800, Guan Jing wrote:
>> If CONFIG_HUGETLB_PAGE is not set and CONFIG_FS_DAX=y.
>> Use command "make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-" to
>> build, will fail:
>>
>> mm/memory-failure.c:568:13: error: ‘collect_procs_fsdax’ defined
>> but not used [-Werror=unused-function]
>> static void collect_procs_fsdax(struct page *page,
>> ^~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make[1]: *** [mm/memory-failure.o] Error 1
>> make[1]: *** Waiting for unfinished jobs....
>> make: *** [mm] Error 2
>> make: *** Waiting for unfinished jobs....
>>
>> so extending "#ifdef CONFIG_HUGETLB_PAG" to cover
>> collect_procs_fsdax() would be a simple resolution.
>
> Thank you for reporting. I think that collect_procs_fsdax() seems to not
> depend on hugetlb, so probably mf_dax_kill_procs() should be defined outside
> "#ifdef CONFIG_HUGETLB_PAGE"?
I tend to agree with Naoya. CONFIG_FS_DAX should not depend on CONFIG_HUGETLB_PAGE.
There are some functions, e.g. mf_generic_kill_procs, are under the CONFIG_HUGETLB_PAGE
unnecessarily. Those should be cleaned up altogether.
Thanks!
>
> Thanks,
> Naoya Horiguchi
>
>>
>> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
>> Fixes: 143ac5cd7289 ("mm: introduce mf_dax_kill_procs() for fsdax case")
>> Signed-off-by: Guan Jing <guanjing6@xxxxxxxxxx>
>> ---
>> mm/memory-failure.c | 56 ++++++++++++++++++++++-----------------------
>> 1 file changed, 28 insertions(+), 28 deletions(-)
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index 545f402c4890..952f117e4584 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -561,34 +561,6 @@ static void collect_procs_file(struct page *page, struct list_head *to_kill,
>> i_mmap_unlock_read(mapping);
>> }
>>
>> -#ifdef CONFIG_FS_DAX
>> -/*
>> - * Collect processes when the error hit a fsdax page.
>> - */
>> -static void collect_procs_fsdax(struct page *page,
>> - struct address_space *mapping, pgoff_t pgoff,
>> - struct list_head *to_kill)
>> -{
>> - struct vm_area_struct *vma;
>> - struct task_struct *tsk;
>> -
>> - i_mmap_lock_read(mapping);
>> - read_lock(&tasklist_lock);
>> - for_each_process(tsk) {
>> - struct task_struct *t = task_early_kill(tsk, true);
>> -
>> - if (!t)
>> - continue;
>> - vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
>> - if (vma->vm_mm == t->mm)
>> - add_to_kill(t, page, pgoff, vma, to_kill);
>> - }
>> - }
>> - read_unlock(&tasklist_lock);
>> - i_mmap_unlock_read(mapping);
>> -}
>> -#endif /* CONFIG_FS_DAX */
>> -
>> /*
>> * Collect the processes who have the corrupted page mapped to kill.
>> */
>> @@ -1540,6 +1512,34 @@ static int try_to_split_thp_page(struct page *page, const char *msg)
>> }
>>
>> #ifdef CONFIG_HUGETLB_PAGE
>> +#ifdef CONFIG_FS_DAX
>> +/*
>> + * Collect processes when the error hit a fsdax page.
>> + */
>> +static void collect_procs_fsdax(struct page *page,
>> + struct address_space *mapping, pgoff_t pgoff,
>> + struct list_head *to_kill)
>> +{
>> + struct vm_area_struct *vma;
>> + struct task_struct *tsk;
>> +
>> + i_mmap_lock_read(mapping);
>> + read_lock(&tasklist_lock);
>> + for_each_process(tsk) {
>> + struct task_struct *t = task_early_kill(tsk, true);
>> +
>> + if (!t)
>> + continue;
>> + vma_interval_tree_foreach(vma, &mapping->i_mmap, pgoff, pgoff) {
>> + if (vma->vm_mm == t->mm)
>> + add_to_kill(t, page, pgoff, vma, to_kill);
>> + }
>> + }
>> + read_unlock(&tasklist_lock);
>> + i_mmap_unlock_read(mapping);
>> +}
>> +#endif /* CONFIG_FS_DAX */
>> +
>> /*
>> * Called from hugetlb code with hugetlb_lock held.
>> *
>> --
>> 2.17.1
