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 944A9C433EF for ; Wed, 26 Jan 2022 23:09:58 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE7316B0071; Wed, 26 Jan 2022 18:09:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E94DF6B0072; Wed, 26 Jan 2022 18:09:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D367E6B0073; Wed, 26 Jan 2022 18:09:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0170.hostedemail.com [216.40.44.170]) by kanga.kvack.org (Postfix) with ESMTP id C227B6B0071 for ; Wed, 26 Jan 2022 18:09:57 -0500 (EST) Received: from smtpin13.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 7789C8249980 for ; Wed, 26 Jan 2022 23:09:57 +0000 (UTC) X-FDA: 79073982834.13.FFA45D2 Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com [209.85.222.176]) by imf26.hostedemail.com (Postfix) with ESMTP id 10078140002 for ; Wed, 26 Jan 2022 23:09:56 +0000 (UTC) Received: by mail-qk1-f176.google.com with SMTP id 71so1071924qkf.4 for ; Wed, 26 Jan 2022 15:09:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:from:to:cc:subject:in-reply-to:message-id:references :mime-version; bh=IVxNWH/QKGOB3lBXSiTvC5/myLAVD8oMrbCJbkN8fLI=; b=lwW5yJNzRuS7LA0HVMmPlkxlFXrgLJLHSmf++01SgGkwUYEjuu3hhD7Pas0rcQaY8C Mj+m8EWSylqe08j5KDrsT51qFkms+qhF14tzMeUxBgrtBaGzu7fgqLdpbTiFYDho4pgh Bv+VeiLJxs+z41wBFEwZMSgZIJQVX1QeOMHCAx3E7A1siJcyImMcYbOoltgc3pv8oqXa VLC/imZ1nAvWGajj0Yk/yJXRVPl/7iuaKrgG7+cOvkY2MEOKrZX50WdcZwea6yw4Ihsy DLuN3KHFg7eTPJWUtnFPi4dOYiyrn8O1qFafJ4Fk4hRCLmHIrXGOr9kjqgctyR4ChLHw 4WeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=IVxNWH/QKGOB3lBXSiTvC5/myLAVD8oMrbCJbkN8fLI=; b=CXDa3346uKznhaCqPn8E3FmXQqmBpqYm0miTLj8XPmRsS/2+mUNAf2FpBCvkMNiYIh Ybj5Jf9/XEDU84KT1Vk7AkcjvC5x1AJrs8fKCN/QP+t+VZU2xthuw37PuHeoUJcsTDeZ WRMA61K2YoiA8B/0JLWgYduMUmhX0fxPxZSgQfPnWzeQsAQFq0SPl8ubywL0JlulzU9Y 5BjUcTLBPF0Vvpts2fhId7h5+7Rsi70kHtZCY2WBaHJB+QNlozHlZo8RsJtvM43esVLz 1a1bEDpwAF/qYyFl3G4Rn/qwdoszbsfPnpL8y3vC+RQxSbInH7kq19XkYjsEz7F+uvPR UbQQ== X-Gm-Message-State: AOAM530W3sD5PnXLsH159m2lXjeo2AXH0EJYUbfVpTgx/P2i1zTSmPN7 bsalJoNvMX/N+uyS8vuZPTDDxw== X-Google-Smtp-Source: ABdhPJx0PSz7sx70cDEfKSN/XtlqIkthxJ42AWV4poLFgw2ZAXxGtaO8trHaQfIunIbj5iJyvr+gZw== X-Received: by 2002:a37:9f45:: with SMTP id i66mr785036qke.205.1643238596073; Wed, 26 Jan 2022 15:09:56 -0800 (PST) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id i13sm475717qko.91.2022.01.26.15.09.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jan 2022 15:09:55 -0800 (PST) Date: Wed, 26 Jan 2022 15:09:43 -0800 (PST) From: Hugh Dickins X-X-Sender: hugh@ripple.anvils To: NeilBrown cc: Christoph Hellwig , Trond Myklebust , Anna Schumaker , Chuck Lever , Andrew Morton , Mel Gorman , David Howells , linux-nfs@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 02/23] MM: extend block-plugging to cover all swap reads with read-ahead In-Reply-To: <164323362698.5493.8309546969459514762@noble.neil.brown.name> Message-ID: References: <164299573337.26253.7538614611220034049.stgit@noble.brown>, <164299611274.26253.13900771841681128440.stgit@noble.brown>, <164323362698.5493.8309546969459514762@noble.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 10078140002 X-Stat-Signature: 5hj49hqtx3otdu7wnwzbipqkkm149cdt Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=lwW5yJNz; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf26.hostedemail.com: domain of hughd@google.com designates 209.85.222.176 as permitted sender) smtp.mailfrom=hughd@google.com X-Rspam-User: nil X-HE-Tag: 1643238596-976426 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 Thu, 27 Jan 2022, NeilBrown wrote: > On Mon, 24 Jan 2022, Christoph Hellwig wrote: > > On Mon, Jan 24, 2022 at 02:48:32PM +1100, NeilBrown wrote: > > > Code that does swap read-ahead uses blk_start_plug() and > > > blk_finish_plug() to allow lower levels to combine multiple read-ahead > > > pages into a single request, but calls blk_finish_plug() *before* > > > submitting the original (non-ahead) read request. > > > This missed an opportunity to combine read requests. No, you're misunderstanding there. All the necessary reads are issued within the loop, between the plug and unplug: it does not skip over the target page in the loop, but issues its read along with the rest. But it has not kept any of those pages locked, nor even kept any refcounts raised: so at the end has to look up the target page again with the final read_swap_cache_async() (which also copes with the highly unlikely case that the page got swapped out again meanwhile). > > > > > > This patch moves the blk_finish_plug to *after* all the reads. > > > This will likely combine the primary read with some of the "ahead" > > > reads, and that may slightly increase the latency of that read, but it > > > should more than make up for this by making more efficient use of the > > > storage path. > > > > > > The patch mostly makes the code look more consistent. Performance > > > change is unlikely to be noticeable. > > > > Looks good: > > > > Reviewed-by: Christoph Hellwig > > Thanks. > > > > > Fixes-no-auto-backport: 3fb5c298b04e ("swap: allow swap readahead to be merged") > > > > Is this really a thing? > Maybe it should be..... > As I'm sure you guessed, I think it is valuable to record this > connection between commits, but I don't like it hasty automatic > backporting of patches where the (unknown) risk might exceed the (known) > value. This is how I choose to state my displeasure. I don't suppose your patch does any actual harm (beyond propagating a misunderstanding), but it's certainly not a fix, and I think should simply be dropped from the series. (But please don't expect any comment from me on the rest: SWP_FS_OPS has always been beyond my understanding.) Hugh