linux-kernel - Re: [RFC PATCH v3 4/9] mm, fsdax: Refactor memory-failure handler for dax mapping

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Dec 2020 09:48:34 +0800
From:   Ruan Shiyang <ruansy.fnst@...fujitsu.com>
To:     Dave Chinner <david@...morbit.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-xfs@...r.kernel.org>,
        <linux-nvdimm@...ts.01.org>, <linux-mm@...ck.org>,
        <linux-fsdevel@...r.kernel.org>, <linux-raid@...r.kernel.org>,
        <darrick.wong@...cle.com>, <dan.j.williams@...el.com>,
        <hch@....de>, <song@...nel.org>, <rgoldwyn@...e.de>,
        <qi.fuli@...itsu.com>, <y-goto@...itsu.com>
Subject: Re: [RFC PATCH v3 4/9] mm, fsdax: Refactor memory-failure handler for
 dax mapping



On 2020/12/17 上午5:26, Dave Chinner wrote:
> On Tue, Dec 15, 2020 at 08:14:09PM +0800, Shiyang Ruan wrote:
>> The current memory_failure_dev_pagemap() can only handle single-mapped
>> dax page for fsdax mode.  The dax page could be mapped by multiple files
>> and offsets if we let reflink feature & fsdax mode work together.  So,
>> we refactor current implementation to support handle memory failure on
>> each file and offset.
>>
>> Signed-off-by: Shiyang Ruan <ruansy.fnst@...fujitsu.com>
>> ---
> .....
>>   static const char *action_name[] = {
>> @@ -1147,6 +1148,60 @@ static int try_to_split_thp_page(struct page *page, const char *msg)
>>   	return 0;
>>   }
>>   
>> +int mf_dax_mapping_kill_procs(struct address_space *mapping, pgoff_t index, int flags)
>> +{
>> +	const bool unmap_success = true;
>> +	unsigned long pfn, size = 0;
>> +	struct to_kill *tk;
>> +	LIST_HEAD(to_kill);
>> +	int rc = -EBUSY;
>> +	loff_t start;
>> +	dax_entry_t cookie;
>> +
>> +	/*
>> +	 * Prevent the inode from being freed while we are interrogating
>> +	 * the address_space, typically this would be handled by
>> +	 * lock_page(), but dax pages do not use the page lock. This
>> +	 * also prevents changes to the mapping of this pfn until
>> +	 * poison signaling is complete.
>> +	 */
>> +	cookie = dax_lock(mapping, index, &pfn);
>> +	if (!cookie)
>> +		goto unlock;
> 
> Why do we need to prevent the inode from going away here? This
> function gets called by XFS after doing an xfs_iget() call to grab
> the inode that owns the block. Hence the the inode (and the mapping)
> are guaranteed to be referenced and can't go away. Hence for the
> filesystem based callers, this whole "dax_lock()" thing can go away >
> So, AFAICT, the dax_lock() stuff is only necessary when the
> filesystem can't be used to resolve the owner of physical page that
> went bad....

Yes, you are right.  I made a mistake in the calling sequence here. 
Thanks for pointing out.


--
Thanks,
Ruan Shiyang.

> 
> Cheers,
> 
> Dave.
>