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=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham 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 6E71DC4338F for ; Fri, 6 Aug 2021 05:21:30 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id D8B95610FC for ; Fri, 6 Aug 2021 05:21:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D8B95610FC Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 2731B8D0001; Fri, 6 Aug 2021 01:21:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2232A6B0071; Fri, 6 Aug 2021 01:21:29 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1130C8D0001; Fri, 6 Aug 2021 01:21:29 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0101.hostedemail.com [216.40.44.101]) by kanga.kvack.org (Postfix) with ESMTP id E889C6B006C for ; Fri, 6 Aug 2021 01:21:28 -0400 (EDT) Received: from smtpin36.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 84498180AE7E1 for ; Fri, 6 Aug 2021 05:21:28 +0000 (UTC) X-FDA: 78443507856.36.D3341D8 Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) by imf22.hostedemail.com (Postfix) with ESMTP id 3A745B243 for ; Fri, 6 Aug 2021 05:21:28 +0000 (UTC) Received: by mail-qv1-f49.google.com with SMTP id em4so4353086qvb.0 for ; Thu, 05 Aug 2021 22:21:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :mime-version; bh=zcQ8sL2CqDTX+4kbgN4gpZBAnX2ERdxEAX8JcULAxIw=; b=gUNzJSOkj20KGKHXFPVIgJjwzfkmpuadqQVQpncG4R9KzZkgDK7F33mliEcdzrl5ep 8A2fWKG5yycXn0Wm0gXcCfdmVRENQuZGBzUcUTbIjoymlifLcaASCmqk34KrHyI7k5wZ /wjyfSaCTB9U7DgfOrP5kz4ACwISxt/Nl1cXRSqkGnEu/LWRcJfBD+gAZ96FE551038H chdRHjClBWCNo80PpjOw1TXVOvt3UcianthQ0Fd4q8id+rjkVB3F48iuzM3AKao/OL2t phCu6ig+mjf+7opPXQB86ugRjpibTknZgynHrR2aY+DNUZ5NVBNiXx6CIXOSLc15/TZY LyYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=zcQ8sL2CqDTX+4kbgN4gpZBAnX2ERdxEAX8JcULAxIw=; b=XhbBfLafLGhR9c2NEBA8zYhSUhPSyYfdqAVK8WkozYdUt+9qQyvIpLHOJDq8AGWI/t MsD2CVlGnCLGU72m7p/xNt5nn4JJAdYQHnPvriMNXvFSj2NmGGma48FB7LdK03pRDgTc qyIZKncrg/9RUZjorTJllTM8nRCcc4y7MsXEofUF8wSBfkQ4oMnMz7k4vaajJy8jiLMi BQVfpRFPQYmpF3YVGl67tj1nQRtSklTt6Apk2QG2tDDHS6lxaHtziwlBdvDkeDO/cFoa gNt5oJmljYcD0u56Y8ihfCSichLEvlHIO22wcVtyMFLwkpUZXJFwKdUCeVwaSs7z+qis CVzQ== X-Gm-Message-State: AOAM530N2VivII2I0ZYu8Q81sdYThiSMm3YWyXz92KykbHu4t7OYveZV sTEeLftaZfUQBGxedYGjOoB25Q== X-Google-Smtp-Source: ABdhPJx2TIa3URYgDE8tF8OrtnfZbT+pjWok+3pEMPuWe/4OFj/lOx3ydolPsjWS9Et4I5nTeiWVeQ== X-Received: by 2002:ad4:4312:: with SMTP id c18mr9113165qvs.54.1628227287282; Thu, 05 Aug 2021 22:21:27 -0700 (PDT) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id q3sm4181366qkn.14.2021.08.05.22.21.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Aug 2021 22:21:26 -0700 (PDT) Date: Thu, 5 Aug 2021 22:21:24 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@ripple.anvils To: Yang Shi cc: Hugh Dickins , Andrew Morton , Shakeel Butt , "Kirill A. Shutemov" , Miaohe Lin , Mike Kravetz , Michal Hocko , Rik van Riel , Christoph Hellwig , Matthew Wilcox , "Eric W. Biederman" , Alexey Gladkov , Chris Wilson , Matthew Auld , Linux FS-devel Mailing List , Linux Kernel Mailing List , linux-api@vger.kernel.org, Linux MM Subject: Re: [PATCH 06/16] huge tmpfs: shmem_is_huge(vma, inode, index) In-Reply-To: Message-ID: <5add2467-3b23-f8b8-e07b-82d8a573ecb7@google.com> References: <2862852d-badd-7486-3a8e-c5ea9666d6fb@google.com> <8baad8b2-8f7a-2589-ce21-4135a59c5dc6@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 3A745B243 Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=gUNzJSOk; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf22.hostedemail.com: domain of hughd@google.com designates 209.85.219.49 as permitted sender) smtp.mailfrom=hughd@google.com X-Stat-Signature: 165fbn7g1knxwtjygj4z4w6gncx4a14j X-HE-Tag: 1628227288-242439 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, 4 Aug 2021, Yang Shi wrote: > On Wed, Aug 4, 2021 at 1:28 AM Hugh Dickins wrote: > > > > Thanks, but despite us agreeing that the race is too unlikely to be worth > > optimizing against, it does still nag at me ever since you questioned it: > > silly, but I can't quite be convinced by my own dismissals. > > > > I do still want to get rid of SGP_HUGE and SGP_NOHUGE, clearing up those > > huge allocation decisions remains the intention; but now think to add > > SGP_NOALLOC for collapse_file() in place of SGP_NOHUGE or SGP_CACHE - > > to rule out that possibility of mischarge after racing hole-punch, > > no matter whether it's huge or small. If any such race occurs, > > collapse_file() should just give up. > > > > This being the "Stupid me" SGP_READ idea, except that of course would > > not work: because half the point of that block in collapse_file() is > > to initialize the !Uptodate pages, whereas SGP_READ avoids doing so. > > > > There is, of course, the danger that in fixing this unlikely mischarge, > > I've got the code wrong and am introducing a bug: here's what a 17/16 > > would look like, though it will be better inserted early. I got sick > > of all the "if (page "s, and was glad of the opportunity to fix that > > outdated "bring it back from swap" comment - swap got done above. > > > > What do you think? Should I add this in or leave it out? > > Thanks for keeping investigating this. The patch looks good to me. I > think we could go this way. Just a nit below. Thanks, I'll add it into the series, a patch before SGP_NOHUGE goes away; but I'm not intending to respin the series until there's more feedback from others - fcntl versus fadvise is the main issue so far. > > --- a/include/linux/shmem_fs.h > > +++ b/include/linux/shmem_fs.h > > @@ -108,6 +108,7 @@ extern unsigned long shmem_partial_swap_usage(struct address_space *mapping, > > /* Flag allocation requirements to shmem_getpage */ > > enum sgp_type { > > SGP_READ, /* don't exceed i_size, don't allocate page */ > > + SGP_NOALLOC, /* like SGP_READ, but do use fallocated page */ > > The comment looks misleading, it seems SGP_NOALLOC does clear the > Uptodate flag but SGP_READ doesn't. Or it is fine not to distinguish > this difference? I think you meant to say, SGP_NOALLOC does *set* the Uptodate flag but SGP_READ doesn't. And a more significant difference, as coded to suit collapse_file(), is that SGP_NOALLOC returns failure on hole, whereas SGP_READ returns success: I should have mentioned that. When I wrote "like SGP_READ" there, I just meant "like what's said in the line above": would "ditto" be okay with you, and I say SGP_NOALLOC, /* ditto, but fail on hole, or use fallocated page */ I don't really want to get into the "Uptodate" business there. And I'm afraid someone is going to ask me to write multi-line comments on each of those SGP_flags, and I'm going to plead "read the source"! Oh, now I see why you said SGP_NOALLOC does clear the Uptodate flag: "goto clear", haha: when we clear the page we set the Uptodate flag. And I may have another patch to slot in: I was half expecting you to question why SGP_READ behaves as it does, so in preparing its defence I checked, and found it was not doing quite what I remembered: changes were made a long time ago, which have left it slightly suboptimal. But that really has nothing to do with the rest of this series, and I don't need to run it past you before reposting. I hope that some of the features in this series can be useful to you. Thanks, Hugh