Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <79f79813-bb9a-bfff-0837-a24d14da0b19@oracle.com>
Date:   Fri, 4 Mar 2022 11:32:02 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH 1/4] mm/memory-failure.c: fix race with changing page
 compound again
Content-Language: en-US
To:     =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= 
        <naoya.horiguchi@nec.com>, Miaohe Lin <linmiaohe@huawei.com>
Cc:     "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20220228140245.24552-1-linmiaohe@huawei.com>
 <20220228140245.24552-2-linmiaohe@huawei.com>
 <20220304082624.GA3778609@hori.linux.bs1.fc.nec.co.jp>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <20220304082624.GA3778609@hori.linux.bs1.fc.nec.co.jp>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dHdicjhOYmI2eUdSUnFqdFNBM0VxVlZNNTFXdnlySjFQY25rTUJnc2tWV1Bt?=
 =?utf-8?B?NWxZaGhvdGd1QXRyaVVkcWdIWmd4RldKK0xRVXhxeTFIZmF3RlhSUituNlJp?=
 =?utf-8?B?L3dGQTdiQVlmUVdsdkdiMXVtektMMklsS1ZkckJJWmNjc21WSTFmZEpyaEhm?=
 =?utf-8?B?M0piRUYxMnJGbW5WdFpzamxzaUJqVUNJbFpuc0R1MSsvNEV6c2pRR3RhUkFT?=
 =?utf-8?B?MWhUcmxJWEFGMUdkV0hwSjY3d0lWT2tRS3FBa3VmVXlJbDNjNXplL2ZnM2t5?=
 =?utf-8?B?RFhXb0NzNmQxWlVreGE4eUFDdmZHLy9ZUWJWRGpyMDdqM1lERjBmRHB6QktM?=
 =?utf-8?B?amlZUHc3azVMc2hFdVNNWUMxZFNoempwVEpBK094TllSOWR5bDcxSDlSaEp5?=
 =?utf-8?B?R0k2cEN3WjFScmVWeXlxU28yaCs0bUp2ZlpGRUZwU2dpeXFXYWUwUHB1Y25D?=
 =?utf-8?B?NzdTM0hTQnJuWVA4WkxtR0lSTW1jUnBKWWEyWUkrRjRaTEwvUkhudW5nU2x6?=
 =?utf-8?B?UDJqa0ZNRWRQSUUrYjRpVlVIS0UwTXZwT1JDOWVJSjd4dUg4Ri9jaElkRkhN?=
 =?utf-8?B?UG5CM1RMeU5LRm0zaEhJUEtxejh2UXJ3cUxkSU0vQ1UwN3F3aFhIa2xvcXlh?=
 =?utf-8?B?NzVqU3V2YmMzZ3AyKzZBL0tRclhSaDJOK0dUUUhkR0RHNzZpM0RQWlBHdTI4?=
 =?utf-8?B?dkJtdng5OUFHZlQ4azJjNGFkeWJmMWVVRHpQdkd4U2VHSXdMOXU1WFpoNVFS?=
 =?utf-8?B?ckNyVUZnSm8rRHd6NUhQMnJxZ3RtMG5LQ1phc2RLNG1WMDlkUm13RUJxS2dm?=
 =?utf-8?B?ZTdlZTNnYmpGWGt6ejMranAxRzlja3dhd2JXTVdWc2Erc1JJeE9ock9hL3M1?=
 =?utf-8?B?cXFmZU4zdUJiMk5FTEt0aUxjbGdsWi9LY1NKSXF2aitrck5kalgxMGpvVnp5?=
 =?utf-8?B?eEdNLzhRMlUveHVjcjNJYnhCME1rWi80alM4Uk15QXNZZkR1bFZmcDVBWlNk?=
 =?utf-8?B?aTRNRE5nMnFYaVVKbjVoMEhBS2x1WmpNcGt2NGtCTUdwdXl1Y0VTajArWVA5?=
 =?utf-8?B?c2F5T2N6M2wwQXBDQVJORm9iSU8zY0Y5SjE4bWFCOGFoOWRISFVWQTVZNkYz?=
 =?utf-8?B?d1Z3dE5LdVR2bksydHJCVVd1U0tGR3VzbmIxUG1BVDR6dkkrUS83ZERGWWhk?=
 =?utf-8?B?YnZjZFFHUE5UekFsd1FidzA5QzBJRUFlQ0hjdXFrQVVGb3NhdnhpVkloZVFH?=
 =?utf-8?B?dWRpZTVJQ3dSUk5ncEFHRnBoMXJ5WHlMR205Rk4zTUVLT3c3U2hRKzhlSjJh?=
 =?utf-8?B?UFVReS9FbHdyazBxanQzNUpXVy9KaGhqa0M4UWJneGVORHEyNU54SDhqQUIz?=
 =?utf-8?B?MXNRQ0FMNkc3Y09oeUdpdVpPK2dQb0ZPd2VxZkgveU5uTTk0enJRazFPWGE2?=
 =?utf-8?B?TzhGQTYrRzdLODVNeUFsUHd6NmFVL0E2U0NyOXd4TDRPUnlIT1pnSU1tL1Jl?=
 =?utf-8?B?c0hra1dXajIxMFpUMVpYdXI4MUJCSUpUVlF2eWVZeVNMcU8vMkQyWjRSdjcx?=
 =?utf-8?B?N2hWM09LeXM1MUVoVytkeGtadkh5NVQxbzZzRkxNNXhkZFE4UjFXNXphMWcz?=
 =?utf-8?B?cVprOEdlNEJuUXl3L2FhcDVRTXdhTk9BZVo4SDN0ZVpCem1pRTFRTDFWK0tj?=
 =?utf-8?B?VG54bFZpdGpIMGpaNlowRTBZajIyREZ5QmRsMWkydjM5VUlTbnIwNitMdWdI?=
 =?utf-8?B?a0RWNGVSZHNiQ1orRFgyRFk2QTlST2ZJK3NKc3h2RkI4cEFTenVOYllFU2Vq?=
 =?utf-8?B?UWhnQUdTZDZKM1F1NDN3U1ZZbHNOdnlKZThIV1RSUTU1akVIblpsSG5Ya0VV?=
 =?utf-8?B?R1VWYzhqc1VaTFhZcXd1Mkk2OEV2aUxlbW1YbFNLWFJwbXllMUNvbUpoQnpy?=
 =?utf-8?B?T3IzdkozTWxvSWtnZVlKMjI1b1A2RldIK2ZQTWZxcXI5MUhucmtzaVZJbXhN?=
 =?utf-8?B?SVVQNWI5UFFBWEZocW85Y3puWk5aTlh6UHpXRHFiRWlpQzFuSk9sd3lveHBT?=
 =?utf-8?B?MFhMTEo5WDZkNHJPVVlwUmxrYzZSelBQTkpPNkNkcXlKM3h4MFZHRy9GZXBM?=
 =?utf-8?B?NUdOMnlIQVQwYXhsQU1lc2Nuci9lZVpWRTFnWnlQNnJSRzNDS1VtVnFWY1Nj?=
 =?utf-8?Q?6Hqcw/GqBj0N90ecZ1sBZSU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 83033292-a5c9-4748-ce8e-08d9fe15aa1f
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Mar 2022 19:32:05.2708
 (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: vt6JD1+LHL0v7I5FGSUEuukKPYqnA/cmr5ouYIuqFsRTTZ0nq2+94+nq9jZnLyv0tDJzoH5oJYhwr9BH+xB35Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4596
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10276 signatures=690470
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 adultscore=0 mlxscore=0
 malwarescore=0 bulkscore=0 suspectscore=0 mlxlogscore=999 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2203040096
X-Proofpoint-ORIG-GUID: dH80FtfpB8096Q0Hcy55ptt7IQb00p4z
X-Proofpoint-GUID: dH80FtfpB8096Q0Hcy55ptt7IQb00p4z
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/4/22 00:26, HORIGUCHI NAOYA(堀口 直也) wrote:
> On Mon, Feb 28, 2022 at 10:02:42PM +0800, Miaohe Lin wrote:
>> There is a race window where we got the compound_head, the hugetlb page
>> could be freed to buddy, or even changed to another compound page just
>> before we try to get hwpoison page. If this happens, just bail out.
> 
> I think that when some hugetlb page is about to change into other type/size
> of compound page, it has to go through buddy allocator because hugetlb pages
> are maintained in separate memory allocator and they never change into other
> normal state directly.  memory_failure_hugetlb() takes refcount before
> lock_page(), so the hugetlb page seems not change between get_hwpoison_page()
> and lock_page(). So it this new check really necessary?

A hugetlb page could change size without going through buddy via the new
demote functionality [1].  Only hugetlb pages on the hugetlb free list can
be demoted.  

We should not demote a page if poison is set.  However, there is no check in
the demote code.  IIUC, poison is set early in the memory error handling
process, even before taking ref on page.  Demote code needs to be fixed so
that poisoned pages are not demoted.  I can do that.

With this change in place, then I think Naoya's statement that hugetlb pages
can not change state is correct and this patch is not necessary.

Does that sound reasonable?

[1] https://lore.kernel.org/linux-mm/20211007181918.136982-1-mike.kravetz@oracle.com/
-- 
Mike Kravetz