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 73898C7618E for ; Wed, 26 Apr 2023 21:27:24 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F2DFF6B0130; Wed, 26 Apr 2023 17:27:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EDF3B6B0132; Wed, 26 Apr 2023 17:27:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DA5EC6B0133; Wed, 26 Apr 2023 17:27:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id C7E8E6B0130 for ; Wed, 26 Apr 2023 17:27:23 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 7709CA03D7 for ; Wed, 26 Apr 2023 21:27:23 +0000 (UTC) X-FDA: 80724828366.10.4AAA3C7 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf19.hostedemail.com (Postfix) with ESMTP id 925191A0007 for ; Wed, 26 Apr 2023 21:27:20 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=YZkEEswW; spf=none (imf19.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682544441; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=P+zvqaA+N/jOdHwRNjllwVTW5FbdEn0G9xqqOFN5Erw=; b=mH8m0iR4qOXc7FAnzf7M+DCNv03/hbbVFMuwS6uzYflmVPPD3kem+ACAC9V19Y/JHS1Ute 0zow1HkaLoTRYL4n/F+wxTH5MNVpJLbl5VgFveTzpxVHwJJGZgrwSZ9Xby7yXWcgliDvup +fDKphVYIcp7Nkp8YUAB56cP+fdnfpc= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=YZkEEswW; spf=none (imf19.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682544441; a=rsa-sha256; cv=none; b=ZLaJ6hKEWtDggeYIQleBFl0Ds71TBNCa2cikIOKqnZBi2HbKxoE/wAWQqiHrbcRrBzuwc3 ZhMbL9WZqOdrAemmWc5ADnXQ7g/j2IplBD1OiFE24Ibn9juCWcEjIYs1mt6CndPAU4yZ8l 7B+8nSMnHN1GoGUrGNqTHmMF1G1iiFQ= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=P+zvqaA+N/jOdHwRNjllwVTW5FbdEn0G9xqqOFN5Erw=; b=YZkEEswWkyh/1QDUlVD4KYOc6Q GV0aIkxVeSJXRKI1c8eNMHPAWmzPxTkL/BKuHJPNhxtZ+q8qNL+PyHXsLpJuTSCdNHXKEQu0bqNg6 T6f3w/dMqoZ0iMJqSgd0vsDV4yLdQqQ55CvpQQ//LPpJCHDf/n9L5wcAfnqJv47Pq4CFP/UPwIDMt jmMGVLufXfkAHqZRTS2yzCWg1t/1qwelNdHzd0I+x7AGaDLRjmVY4yzQjiBfiNvFVmqkDs8G5Cpuv yqxd+XSkB3ooM6JWhXkyEQc+uXn5+WaM/boTMOnEbU59GInIj/3R6n4ufg7dqLWOQTlhwgqAf6HCm 72Qz1fbA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1prmf9-002rdk-Tq; Wed, 26 Apr 2023 21:26:56 +0000 Date: Wed, 26 Apr 2023 22:26:55 +0100 From: Matthew Wilcox To: Doug Anderson Cc: Mel Gorman , Hillf Danton , Andrew Morton , Alexander Viro , Christian Brauner , Linus Torvalds , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Yu Zhao Subject: Re: [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout() Message-ID: References: <20230421221249.1616168-1-dianders@chromium.org> <20230421151135.v2.1.I2b71e11264c5c214bc59744b9e13e4c353bc5714@changeid> <20230422051858.1696-1-hdanton@sina.com> <20230425010917.1984-1-hdanton@sina.com> <20230426100918.ku32k6mqoogsnijn@techsingularity.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Stat-Signature: 54efin3tjc86pef87u347jcimtaz9ejs X-Rspam-User: X-Rspamd-Queue-Id: 925191A0007 X-Rspamd-Server: rspam06 X-HE-Tag: 1682544440-32061 X-HE-Meta: U2FsdGVkX19el4LgvzNgN6OsaDXGMDtb/tC6U9wtNO5zCTmwnTaSqNZ3h4IbG6zycnh32iNZTpWJE+d671j+yTOpcLKnxfU38yP8Pns+rqXf5617odzyQnCVn1WgiY2sva+maDkWdt+4WBjtqCLdhQMHMMEw8bhZkc0J3FEPhfTOdQKNWyS6CGNRx2xcgpnfNdbTGFe+TWlk7vIlFD7Ty5duXHydXjCPx6LI6zDHtG7yaiq37wevf2hpvpfd/iBXRQGtPLnaWsiUmzMQxSEuojwO/YZ++joavEKfZvO1aSTFGiEmGfhrDSmHqlhsegsGttG08R3762NK/Rv6ORfy/TrNiuYUh/waAMxWLl7+C7xiHLPOTUXt8sRNDnwmWa3cFKydZJ4MB7eX8CGrmGdGIhhjp8GjmT1f7z9IhI0hJUVicZwtpoqw4uctFK/S7rUREn10gOQXXYSt0GbcDK5IURJby32wsj8Tdt7BN/atLanDKI9jgijSNoG516JA8bIfa8sQ1Yz/D9BBMKPtahUfEMAa0AQ5OM2CzOKd46kwRgJRhN4UdQsN5l+qW8OgMeIqHDS2WQDFuxG+6pD1Qtf8C45cXCt6ei3V+6ZVrYuzeVUqyrmXtwUN4X/Dqtz5uUjuegmLsTDauZZ0qmJBZ5nacWo0XfWOsHqnChWaUblbHG/bnMnRn5APr44Me9ovP7GZ6GONUe0la/ngw5GjuRjI8wVAUQRrWLd/EIqVGdO1txKVk1p2d4Ue4PwKxZJbi6eAdbME3b7pGy78riND52a0s5o6a8Np+DJtBP6Kbf5ANrmhn/P+ffNUnQvpj8ZBFmm7wzvWrCM8oYmcGXfE0tbIJqtFTRPxMv0SELMk1m7EGih+wYUxrBc+k/zq1GuglVEM5IrDdarUzd8B+74sT8d3vrlGoPzcx604d5PHjELvEUGYMnuf/7Jf60YDIHYSI++14iTdVZjvgbM9u8Kja3e xgPN0szw 9qQP05vGQdkg+TJQD7q9aqkR7WjAI0Ih4hX7/lm9Qc7ZxXM06JcN4v3WEmW9hIAbjRHJ4OQ5ZTfCx8zYoc9A+h0YIOavul68626gjv+lr3vyqlXFEZW/pPeX0lmibvLuVcwl15bSn9+BUSWFTMY9QoUhr0BNTkNMEwbxb255VQ5EgNfR0zbss0JaBJ0mNqXmi+FntvllZwzMaRmsid1eompEK+f5Z5kbJZHfSV9DAEfJx64CoJA7l3lqP1g== 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: On Wed, Apr 26, 2023 at 01:46:58PM -0700, Doug Anderson wrote: > On Wed, Apr 26, 2023 at 8:14 AM Matthew Wilcox wrote: > > > > I'm not generally a fan of lock-with-timeout approaches. I think the > > rationale for this one makes sense, but we're going to see some people > > try to use this for situations where it doesn't make sense. > > Although it won't completely prevent the issue, I could add a comment People don't read comments. > > Hm. If the problem is that we want to wait for the lock unless the > > lock is being held for I/O, we can actually tell that in the caller. > > > > if (folio_test_uptodate(folio)) > > folio_lock(folio); > > else > > folio_trylock(folio); > > > > (the folio lock isn't held for writeback, just taken and released; > > if the folio is uptodate, the folio lock should only be taken for a > > short time; if it's !uptodate then it's probably being read) > > The current place in patch #3 where I'm using folio_lock_timeout() > only calls it if a folio_trylock() already failed [2]. So I guess the > idea would be that if the trylock failed and folio_test_uptodate() > returns 0 then we immediately fail, otherwise we call the unbounded > folio_trylock()? Looking at the actual code, here's what I'd do: +++ b/mm/migrate.c @@ -1156,6 +1156,14 @@ static int migrate_folio_unmap(new_page_t get_new_page, free_page_t put_new_page if (current->flags & PF_MEMALLOC) goto out; + /* + * In "light" mode, we can wait for transient locks (eg + * inserting a page into the page table), but it's not + * worth waiting for I/O. + */ + if (mode == MIGRATE_SYNC_LIGHT && !folio_test_uptodate(folio)) + goto out; + folio_lock(src); } locked = true; > I put some traces in and ran my test and it turns out that in every > case (except one) where the tre initial folio_trylock() failed I saw > folio_test_uptodate() return 0. Assuming my test case is typical, I > think that means that coding it with folio_test_uptodate() is roughly > the same as just never waiting at all for the folio lock in the > SYNC_LIGHT case. In the original discussion of my v1 patch people > didn't like that idea. ...so I think that for now I'm going to keep it > with the timeout flow. I think that means that your specific test is generally going to exercise the case where the lock is held because we're waiting for I/O. That's exactly what you set it up to produce, after all! But it won't affect the cases where the folio lock is being held for other reasons, which your testcase is incredibly unlikely to produce.