Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <10577ef3-57c4-b1eb-cd0f-40518dfd9ad0@oracle.com>
Date:   Thu, 20 Jan 2022 16:09:50 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [PATCH v4 1/3] mm/hwpoison: mf_mutex for soft offline and
 unpoison
Content-Language: en-US
To:     Naoya Horiguchi <naoya.horiguchi@linux.dev>, linux-mm@kvack.org
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        David Hildenbrand <david@redhat.com>,
        Oscar Salvador <osalvador@suse.de>,
        Michal Hocko <mhocko@suse.com>,
        Ding Hui <dinghui@sangfor.com.cn>,
        Tony Luck <tony.luck@intel.com>,
        "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Yang Shi <shy828301@gmail.com>, Peter Xu <peterx@redhat.com>,
        Naoya Horiguchi <naoya.horiguchi@nec.com>,
        linux-kernel@vger.kernel.org
References: <20211115084006.3728254-1-naoya.horiguchi@linux.dev>
 <20211115084006.3728254-2-naoya.horiguchi@linux.dev>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <20211115084006.3728254-2-naoya.horiguchi@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MC9yL0F4N3ZadTdzdFY4eXpYRGRLT00xRzBnajBjdEpwcGdiMHo2RDF4azJI?=
 =?utf-8?B?N2dmVll0cy90TGdMZEVHZTc5RllVTUIycVlhNW1IdXhIZjUrNDliUTdEWmI5?=
 =?utf-8?B?MVdTeVF2SWVuMVlockhjSU8ycDNnZHR6NytQVDBLNlpwWmVwZStxeWw2QmJl?=
 =?utf-8?B?aGJ1SitEeXppdWhuWmJQQ2cwWnNQckovWEtMV3k2aVNMbjlBT04ySjVrelBK?=
 =?utf-8?B?V2hvbUk0QmtUM1dTNU1yNkhlZVF4QUl0Z1dmVXV0QTNEdStkTXZMTHgrRGx6?=
 =?utf-8?B?Q1BzUmIzZjNZNUJHczdVc1RtMHhNeksvUG44enZkdEw4bnBOZEI2S3puQW1a?=
 =?utf-8?B?b1NZcWZXa2lnL1ZFWktBSkM0NFNvdGg0blNmS3hFMjNUNnlJalpMNUlJM1hB?=
 =?utf-8?B?ZEl2TTNvQ05tVUhiWUdjL3pHMTdXUS9CUkJ2Y1BHUGVnU01pNm5kVUo5aTV3?=
 =?utf-8?B?Ykc0aTZtSTFmOUkyU0RUdXJXUVhCNWhUYUpZUHBST0VzRm9qREtlN3B3T2N4?=
 =?utf-8?B?cHVFNkcvQmRIMW9ha3lMQndUV0R2bmZkNi9uMVI0d3R0Q09vR3hKY1RGcW1O?=
 =?utf-8?B?K2FHZk5xeU0xMDVsOXpwRTQ4Syt2Mk1STzhXakY3WENoSW1mSkNDanJwY2hz?=
 =?utf-8?B?b1hUZ2IxV3cwcGRTaGZqaytQSUJVaG9DYWRJaVBUaDd2VzZDYXoyUC9MMzlw?=
 =?utf-8?B?ZDhEQkRpMzFvSmVCb0hGU1hwL3dwQnMvb2I4R0NSaTlBN3JMVVFVYXVGeVFu?=
 =?utf-8?B?NUNBdjhiMFNSdnpkMjFDOFlhdm91NVN5NVRMMWVsZTRjZ2llS0RoMVNMWTJS?=
 =?utf-8?B?UWlGWGkwWUtyOU44bk1TWWc4K2JWVVhOdzk5RkNTaWVuRHZyRzdONW1jREVN?=
 =?utf-8?B?RjFpejJoZHVHWnQrUmoyY2tLV1FUWTFyZlc5eDFHZjc4djFnd2FBYTEzY2w0?=
 =?utf-8?B?RlpnOThIYTkwMXRBOWs0b1ArY0VlRURyNWRWSDlJdkJqbjFhWWovNlRUTE5P?=
 =?utf-8?B?TzNrZ29ERk4rTThvem44dklGREkrQ0FaT0ZNNGR3OGQ1UkNZbWVOMFpCZjhW?=
 =?utf-8?B?K1YybG90KzZQbnBQdTVXakwxaEVoSmMxQmh6WmRTWnYwTDRTSnVQYTU0TDRv?=
 =?utf-8?B?Y1Nzd0VUM1F1azBqZFU3bEtuN21VQ2czdjZhWHcvOWdKNFQrcjkwTDFRaGZG?=
 =?utf-8?B?a1RzUGtnZ0pUUmpyakhjSUN1Z1d2cUIyM1hWcEJxSHZWZWRsbVRzTGhFcER1?=
 =?utf-8?B?dFpMeVA3YUF5WGJrekRPR2I2a2FUZFE2Wm5OT3ZkQWFieFNwNGo1VVkxM01l?=
 =?utf-8?B?czBlN0l6ZEpvK05zWHlRd3lDZGlaSkxCS0sxU2lURy9wTzZZMTROelJFL3pI?=
 =?utf-8?B?ZTJyQkExSmROYzlHOU5kd3ZlNnBCU1lUNjBYUjd2Y0c5TGZIVXkybnFjVmdE?=
 =?utf-8?B?ZWozYUVsZkRrNjhrNWVWcW5maGs5N0pCL2tsVE1SMXJFaWZEUEh0aTF1Zklq?=
 =?utf-8?B?REp3OVVXSFlnTEY3U2RFNlJvRWpVYU9NQ1dwUGkydlRBQ3BodFBFMXRpWkZK?=
 =?utf-8?B?emYwQUhJUVNESlY5a2pDSDRWU1hTN0h3dFZqV1ZwU1Y5cXZUbzY2N2thNHI3?=
 =?utf-8?B?MkVwYUZEbFR3blJFN1ZxZEN3bnFwZ1NiNndPWSsyYTdDWnYyY3NNWUl2THc3?=
 =?utf-8?B?Mjh3QTloK01XbEszbzYrRnpySXVaeXBROCsxaVVzSHM3SnNwektJSm5WSkFK?=
 =?utf-8?B?czEyVUpzWWowa3RKSXlrSGNYYThOUzBvUWlzN1dhekNvTVRZZS9TdFhkRlJm?=
 =?utf-8?B?RXpUQi9qSVdtaGFLaGtYQWljbHkwcmk4ZXBtckRNSUw4dDhhZkJEbVBRamlp?=
 =?utf-8?B?bGx5Snd2aHBQbERpdkpGcjg0dnJFU2ZyeE95WDdzd3ZodE5WaXZibUovVXM4?=
 =?utf-8?B?UG9WU2Z0RHZacnRIYk1SODNncktnSS9tT0dyaS9lWDNOTFgvTklncDQzeVlV?=
 =?utf-8?B?cXpRcy90R25KR3NOeVhGTGhicG52YlZMSmJsQzhFRmJHSGZnMUczWW5EMU5q?=
 =?utf-8?B?ODRpN3ZUWlVleFhDU1FQOHd4bnVQS1F5c3ZCeDhSeVNBM3preXZvdUFtcDdS?=
 =?utf-8?B?NXRqeFlLak9paERNYlZwZnlkOVpRS3FZT0hLY3FycXdRMzN4eTBrc05yV3dI?=
 =?utf-8?Q?hg2QUxi3OXyY4xv8Qw3NT8s=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7979e2a8-c437-49ff-7b72-08d9dc72588b
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jan 2022 00:09:52.1585
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: QE3vDKE0gR/qO+KD0nqTwJ1lkPJE7WS66hbT2BT1eVyqT4K1mThlv7pAp+H28h+fNZ5Lk06GIp5Pin0/VozUQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB5838
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10233 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxscore=0 suspectscore=0
 spamscore=0 mlxlogscore=999 bulkscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201210000
X-Proofpoint-GUID: uz9WxwkjVtT6Nyk6DFZDHYL5hfShwUEY
X-Proofpoint-ORIG-GUID: uz9WxwkjVtT6Nyk6DFZDHYL5hfShwUEY
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/15/21 00:40, Naoya Horiguchi wrote:
> From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> 
> Originally mf_mutex is introduced to serialize multiple MCE events, but
> it is not that useful to allow unpoison to run in parallel with memory_failure()
> and soft offline.  So apply mf_mutex to soft offline and unpoison.
> The memory failure handler and soft offline handler get simpler with this.
> 
> Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> Reviewed-by: Yang Shi <shy828301@gmail.com>
> ---
> ChangeLog v4:
> - fix type in commit description.
> 
> ChangeLog v3:
> - merge with "mm/hwpoison: remove race consideration"
> - update description
> 
> ChangeLog v2:
> - add mutex_unlock() in "page already poisoned" path in soft_offline_page().
>   (Thanks to Ding Hui)
> ---
>  mm/memory-failure.c | 62 +++++++++++++--------------------------------
>  1 file changed, 18 insertions(+), 44 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index e8c38e27b753..d29c79de6034 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c

Thanks for working on this.  I tried to exercise memory error handling for
hugetlb pages and ran into issues addressed by these patches.

> @@ -1507,14 +1507,6 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags)
>  	lock_page(head);
>  	page_flags = head->flags;
>  
> -	if (!PageHWPoison(head)) {
> -		pr_err("Memory failure: %#lx: just unpoisoned\n", pfn);
> -		num_poisoned_pages_dec();
> -		unlock_page(head);
> -		put_page(head);
> -		return 0;
> -	}
> -
>  	/*
>  	 * TODO: hwpoison for pud-sized hugetlb doesn't work right now, so
>  	 * simply disable it. In order to make it work properly, we need
> @@ -1628,6 +1620,8 @@ static int memory_failure_dev_pagemap(unsigned long pfn, int flags,
>  	return rc;
>  }
>  
> +static DEFINE_MUTEX(mf_mutex);

There are only two places other places where PageHWPoison is modified without
the mutex.  They are:
- test_and_clear_pmem_poison
  I 'think' pmem error handling is done separately so this does not apply.
- clear_hwpoisoned_pages
  Called before removing memory (and deleting memmap) to reconcile count
  of poisoned pages.  Should not be an issue and technically I do not think
  the ClearPageHWPoison() is actually needed in this routine.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
-- 
Mike Kravetz