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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, FSL_HELO_FAKE,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 62F75C34031 for ; Tue, 18 Feb 2020 22:28:16 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 14D4D208C4 for ; Tue, 18 Feb 2020 22:28:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="geKjqFFA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 14D4D208C4 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B7A3E6B0007; Tue, 18 Feb 2020 17:28:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B2B146B0008; Tue, 18 Feb 2020 17:28:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CC466B000A; Tue, 18 Feb 2020 17:28:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0240.hostedemail.com [216.40.44.240]) by kanga.kvack.org (Postfix) with ESMTP id 827466B0007 for ; Tue, 18 Feb 2020 17:28:15 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 55CC8181AC9C6 for ; Tue, 18 Feb 2020 22:28:15 +0000 (UTC) X-FDA: 76504687350.08.night23_823af44aa4306 X-HE-Tag: night23_823af44aa4306 X-Filterd-Recvd-Size: 5730 Received: from mail-pj1-f68.google.com (mail-pj1-f68.google.com [209.85.216.68]) by imf25.hostedemail.com (Postfix) with ESMTP for ; Tue, 18 Feb 2020 22:28:14 +0000 (UTC) Received: by mail-pj1-f68.google.com with SMTP id d5so1661465pjz.5 for ; Tue, 18 Feb 2020 14:28:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=i+LdxAL8JFR+HR/vOGUczmLCXvRbYnwHbE5xUaDFUiY=; b=geKjqFFAsj4ztZRTzn3HOzlOUNaKSmz1mqF9/RPgi+MgSWmTjoFIdeY/qi6a9fEpZE 3fsY9nxaj6KkokoZ1HYXGEOgOotI2d7PEdHDkIAUwPDhlSjWaV61P+GmCyH6tqpBizCG thNsjcYUvx5ElJabrWULixdO7k4igLUG8hKgMr1lNBr7EeNtI5i1ZQCV4LMon1Rliv4n mSUz2R36l6us8GUcFoAdPI0ukvBnEPAVulIY5QM+9URphGKKveM1zPKDuYu2kgZCtsc0 RynfQKEAg5jmITVmUxY/87yboeXgvRsXmczMQTrx0UDIqnclCx/StparQnbKNab4r6Zb 1e4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=i+LdxAL8JFR+HR/vOGUczmLCXvRbYnwHbE5xUaDFUiY=; b=WI3i7Oc3P4xQJ0CSPvzSs5jhp465l+Je54rok4z2G297Zfq0C2Y5A28uSx5nL3JSQa s2p8MHBh/asrsztyGG9RoxiEg5upBwcc1M6HvxouMRDKS9ZOmbwkUdTmn6dx5X0IN/8r ZR/NZzWsOSEgIInOeUQ7Nl9S7yK39MFVg3OJ/5Wt/mjpR+vY7/RfZ7T4iJMaE8A2eMCE yM994sRx4zKbLQanShYsLcmP08sLoKMlJ1fGuoVmNqhjcLE4xRiwXPLAetRFm3oVnTwh 8J+1XE/yIc+cCIoXXMKIckT1MltBWYE7vGZ8m6IErd4A7n2F0hegN1ihKRn89gHi8K7J Eazg== X-Gm-Message-State: APjAAAXYlNB53fnsNRwUzAkkJIlk/WVecOq1r1HMkp7jXp1YsUqvGcVa cvXar41VLgpLFNjxK3KVpqA= X-Google-Smtp-Source: APXvYqz/hnJw2htfAoc3R+Ui8uOGDivQ61auUnYVaQ4Be224VAkM49K6bSVFQkNot6s7sjG0F2ms0Q== X-Received: by 2002:a17:902:8647:: with SMTP id y7mr22961347plt.224.1582064893445; Tue, 18 Feb 2020 14:28:13 -0800 (PST) Received: from google.com ([2620:15c:211:1:3e01:2939:5992:52da]) by smtp.gmail.com with ESMTPSA id u4sm4316005pgu.75.2020.02.18.14.28.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Feb 2020 14:28:12 -0800 (PST) Date: Tue, 18 Feb 2020 14:28:10 -0800 From: Minchan Kim To: Jan Kara Cc: Andrew Morton , linux-mm , LKML , Matthew Wilcox , Josef Bacik , Johannes Weiner , Robert Stupp Subject: Re: [PATCH 3/3] mm: make PageReadahead more strict Message-ID: <20200218222810.GA126504@google.com> References: <20200212221614.215302-1-minchan@kernel.org> <20200212221614.215302-3-minchan@kernel.org> <20200217093128.GB12032@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200217093128.GB12032@quack2.suse.cz> User-Agent: Mutt/1.10.1 (2018-07-13) 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 Mon, Feb 17, 2020 at 10:31:28AM +0100, Jan Kara wrote: > On Wed 12-02-20 14:16:14, Minchan Kim wrote: > > PG_readahead flag is shared with PG_reclaim but PG_reclaim is only > > used in write context while PG_readahead is used for read context. > > > > To make it clear, let's introduce PageReadahead wrapper with > > !PageWriteback so it could make code clear and we could drop > > PageWriteback check in page_cache_async_readahead, which removes > > pointless dropping mmap_sem. > > > > Signed-off-by: Minchan Kim > > ... > > > +/* Clear PG_readahead only if it's PG_readahead, not PG_reclaim */ > > +static inline int TestClearPageReadahead(struct page *page) > > +{ > > + VM_BUG_ON_PGFLAGS(PageCompound(page), page); > > + > > + return !PageWriteback(page) || > > + test_and_clear_bit(PG_reclaim, &page->flags); > > +} > > I think this is still wrong - if PageWriteback is not set, it will never > clear PG_reclaim bit so effectively the page will stay in PageReadahead > state! > > The logic you really want to implement is: > > if (PageReadahead(page)) { <- this is your new PageReadahead > implementation > clear_bit(PG_reclaim, &page->flags); > return 1; > } > return 0; > > Now this has the problem that it is not atomic. The only way I see to make > this fully atomic is using cmpxchg(). If we wanted to make this kinda-sorta > OK, the proper condition would look like: > > return !PageWriteback(page) **&&** > test_and_clear_bit(PG_reclaim, &page->flags); > > Which is similar to what you originally had but different because in C '&&' > operator is not commutative due to side-effects committed at sequence points. It's accurate. Thanks, Jan. > > BTW: I share Andrew's view that we are piling hacks to fix problems caused > by older hacks. But I don't see any good option how to unalias > PG_readahead and PG_reclaim. I believe it's okay to remove PG_writeback check in page_cache_async_readahead. It's merely optmization to accelerate page reclaim when the LRU victim page's writeback is done by VM. If we removes the condition, we just lose few pages at the moment for fast reclaim but finally they could be reclaimed in inactive LRU and it would be *rare* in that that kinds of writeback from reclaim context should be not common and doesn't affect system behavior a lot. > > Honza > -- > Jan Kara > SUSE Labs, CR