linux-kernel - Re: [PATCH -next] mm：fix build error of defined but not used

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 9 Jun 2022 15:38:58 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Guan Jing <guanjing6@...wei.com>
CC:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "djwong@...nel.org" <djwong@...nel.org>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "ruansy.fnst@...itsu.com" <ruansy.fnst@...itsu.com>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>
Subject: Re: [PATCH -next] mm：fix build error of defined but not used

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@...wei.com>
>> Fixes: 143ac5cd7289 ("mm: introduce mf_dax_kill_procs() for fsdax case")
>> Signed-off-by: Guan Jing <guanjing6@...wei.com>
>> ---
>>  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