From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BD89EC54E58 for ; Tue, 12 Mar 2024 03:45:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C32688D0013; Mon, 11 Mar 2024 23:45:57 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BE2778D0010; Mon, 11 Mar 2024 23:45:57 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AD1ED8D0013; Mon, 11 Mar 2024 23:45:57 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 9DB148D0010 for ; Mon, 11 Mar 2024 23:45:57 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 266AB120A5B for ; Tue, 12 Mar 2024 03:45:57 +0000 (UTC) X-FDA: 81886998354.05.C79D2C7 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf01.hostedemail.com (Postfix) with ESMTP id 087A640005 for ; Tue, 12 Mar 2024 03:45:53 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=IjG7Y0cQ; dmarc=none; spf=none (imf01.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1710215154; a=rsa-sha256; cv=none; b=DZW3lZqdVGGPwPTTziwWqZpem13kuUAqN14J4qg3U3cnyFOsu/rtjgEuy1pEfsK6Qq5Eg1 mXxjdaeOb7Ptie76KXV4Za9hoh8tP5Z6EDQnNP/MRal/7aD7955ivxdbPyTl5BTvQbwZ3R W0uHCaIy4r9OE86C2nKui6pg+nMlZYI= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=IjG7Y0cQ; dmarc=none; spf=none (imf01.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1710215154; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HxUcv7Fm4MUGTSEA7CZZNMCascC7JNYS1DaeiBPDSXI=; b=4obaqemjJa2PaQ1OrenF+0asw74DYd3fdePa57xHGXfhjM8Nw2uSyAD6fJZO5OaTj5aRh+ okx8f1WrSHZxkoyzBd+fGCfInkVwyco5iK1fW7CcpeZ5Zp/vr5UUdERMSlf2itLKzGboCP w8///s4d5vqD14bseV0w3dZYkB1xe/k= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=HxUcv7Fm4MUGTSEA7CZZNMCascC7JNYS1DaeiBPDSXI=; b=IjG7Y0cQbifNB1bLRHAIVx+7Q8 3ANUIWVtCXBG6LaT7P0n0kvnMRQcsvIjr6EQT5OCBI2DldQ0RRkQyWvLW512CjW6O+YP4FXfi1G6w QecfYDwzFZpxdThmadeNoz0l9c9C7Oe266BUcmLf3ZYJMU8Y+00pkDmDdYWexznClHR2KaABx3oV3 Ndh8pRY2cJe1hwfeVGknNKP6h551DpMnAp6ur0HZDQPgCFR6TNgzd6uma7lznAdc6i7XxrJ7edaCY 8WxAi5OiFH5x7mvoBGBFZcxIPBKIsjDE3iQSyQRby94gQ6L/AtHyQQ2w0z/lSwRHSOmz2aB1M+voE cQWEJjKA==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rjt5H-00000002AFY-2jsz; Tue, 12 Mar 2024 03:45:47 +0000 Date: Tue, 12 Mar 2024 03:45:47 +0000 From: Matthew Wilcox To: Zi Yan Cc: linux-mm@kvack.org, Andrew Morton , Yang Shi , Huang Ying , "Kirill A . Shutemov" , Ryan Roberts , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] mm/migrate: put dest folio on deferred split list if source was there. Message-ID: References: <20240311195848.135067-1-zi.yan@sent.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240311195848.135067-1-zi.yan@sent.com> X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 087A640005 X-Stat-Signature: tpu34r44h7ebjcm51py8zu7jcyna67yc X-HE-Tag: 1710215153-551992 X-HE-Meta: U2FsdGVkX18b7w3e6UTI7yOcu5RFmyCkuXMaXr/MLlnHzEiW15sUFEZpVOuWde5F9th8hr6zhr3RjSVeZjMdl0JXewslxPQNinrP8De7rAWiqrMq7tPRAQ+M3bbzibXM0KZt/wPAenXNWnwUzloiP4f4ckAp/5s45pxNjM/KytQuRjyTSYJk8s5O8jbYsFKjDTms3ArkLcUW0vVXxcu/e13MSwRK+dQxJ7R2TD3N581bOHEite3WiOE4ZcSOqcpp2UYfRVig/sp7dtGkc87HIJzAGbDtrNbFuZVt0pQsOiNP78aHgk1d55PPx3b2nWZuXGa6sbYD0SCJio01v4pNvJtMJXs16QMTg/CQeFARlWmZ2qAbYMNyj0OTmngdMrefSfBPXAMV1XnBF5Z05BC8gedQlqt+DpjKCAuAEJvzrqresFnDDuZv6JdK9A2FVwFwJFQYicULlJlQ7y+efhyC++NRZwOVfxUTfUaXKUTSYrqWE/sMHCLLxJW6mAmVTFVL8ZUE9J7WIih9sgGDad8o7kviu8FW9BpPEXVFULltnwMNNlKk828qfeKwe4+ftkKJrXGSGLBur9BdtTwbmWJL4XNrCU/u77d/I3vv6H7pqYXsWPJrlb2WRi+aemZXlAynW8XcYpUKBDIm0J4PZfnwJIch3wccagUz2GXnfUEdMrBQ2ou8QvOpQ7IBjh7TQ38WFvfMa51XJ/ihZNJMjV86sEFswZ3Gt7J9VyDZWfSuPCJ8jY4c51/2UfcXv29o12HCiG/h2C7G6N4FMeBys1EeK0Gn+rwLydebXtKyfTLArnF/kqioi+cyeyzENbcvUzTeiE6s5SkoxV0KuJYNYlTldQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Mar 11, 2024 at 03:58:48PM -0400, Zi Yan wrote: > @@ -1168,6 +1172,17 @@ static int migrate_folio_unmap(new_folio_t get_new_folio, > folio_lock(src); > } > locked = true; > + if (folio_test_large_rmappable(src) && > + !list_empty(&src->_deferred_list)) { > + struct deferred_split *ds_queue = get_deferred_split_queue(src); > + > + spin_lock(&ds_queue->split_queue_lock); > + ds_queue->split_queue_len--; > + list_del_init(&src->_deferred_list); > + spin_unlock(&ds_queue->split_queue_lock); > + old_page_state |= PAGE_WAS_ON_DEFERRED_LIST; > + } I have a few problems with this ... Trivial: your whitespace is utterly broken. You can't use a single tab for both indicating control flow change and for line-too-long. Slightly more important: You're checking list_empty outside the lock (which is fine in order to avoid unnecessarily acquiring the lock), but you need to re-check it inside the lock in case of a race. And you didn't mark it as data_race(), so KMSAN will whinge. Much more important: You're doing this with a positive refcount, which breaks the (undocumented) logic in deferred_split_scan() that a folio with a positive refcount will not be removed from the list. Maximally important: Wer shouldn't be doing any of this! This folio is on the deferred split list. We shouldn't be migrating it as a single entity; we should be splitting it now that we're in a context where we can do the right thing and split it. Documentation/mm/transhuge.rst is clear that we don't split it straight away due to locking context. Splitting it on migration is clearly the right thing to do. If splitting fails, we should just fail the migration; splitting fails due to excess references, and if the source folio has excess references, then migration would fail too.