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 EA331C3DA4B for ; Wed, 17 Jul 2024 15:26:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 770076B009A; Wed, 17 Jul 2024 11:26:23 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 71FEC6B00A0; Wed, 17 Jul 2024 11:26:23 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5E7916B00A1; Wed, 17 Jul 2024 11:26:23 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 3FE1B6B009A for ; Wed, 17 Jul 2024 11:26:23 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 95FAAA35A2 for ; Wed, 17 Jul 2024 15:26:22 +0000 (UTC) X-FDA: 82349621004.05.252D3DF Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf05.hostedemail.com (Postfix) with ESMTP id 873C8100016 for ; Wed, 17 Jul 2024 15:26:20 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721229928; 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; bh=pAvpo6PwhIjPwsxMyNxaO2Bu1iHuEV0lkhFTjYoNYLM=; b=ry9n4O4Yagp1xVqrMWNEPXmiM0+IWhWqYZALBWcZKXGJC7PSjDcLrAozQAmQsG4mIJ5TYK JF4tUIh52ZjQ677nSFa0mwFgAyBKJR/G4ScTxWIuSk/Pem4VMWOrzyB3ZArGMs57pcJBaP 8wj+jFcXQiqWnWtW9WCJeS3o/5Tp+d8= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721229928; a=rsa-sha256; cv=none; b=8Xzt+8JpcTw3vWfN47g/4D9cT/9eXs+BRrzkvLZodtwgjXPv9Vo49INZ3UF73r72Bi2/iG tGLn+lGKWPIPdFDZkPF2QP+HHIJ8s4+MZlZjpVvZn3nCUVfVm46EWqftViFeAPEXBvv9W8 HkjlfWcqLS2rjkJY57K0SziYQOdMYDU= Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CA402106F; Wed, 17 Jul 2024 08:26:44 -0700 (PDT) Received: from [10.57.77.222] (unknown [10.57.77.222]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9ABCE3F766; Wed, 17 Jul 2024 08:26:16 -0700 (PDT) Message-ID: Date: Wed, 17 Jul 2024 16:26:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v10 01/10] fs: Allow fine-grained control of folio sizes Content-Language: en-GB To: "Pankaj Raghav (Samsung)" Cc: Matthew Wilcox , david@fromorbit.com, chandan.babu@oracle.com, djwong@kernel.org, brauner@kernel.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, yang@os.amperecomputing.com, linux-mm@kvack.org, john.g.garry@oracle.com, linux-fsdevel@vger.kernel.org, hare@suse.de, p.raghav@samsung.com, mcgrof@kernel.org, gost.dev@samsung.com, cl@os.amperecomputing.com, linux-xfs@vger.kernel.org, hch@lst.de, Zi Yan References: <20240715094457.452836-1-kernel@pankajraghav.com> <20240715094457.452836-2-kernel@pankajraghav.com> <20240717094621.fdobfk7coyirg5e5@quentin> <61806152-3450-4a4f-b81f-acc6c6aeed29@arm.com> <20240717151251.x7vkwajb57pefs6m@quentin> From: Ryan Roberts In-Reply-To: <20240717151251.x7vkwajb57pefs6m@quentin> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 873C8100016 X-Stat-Signature: 544ajdo1snbojg4py5dboczscp539ujp X-Rspam-User: X-HE-Tag: 1721229980-983113 X-HE-Meta: U2FsdGVkX1+zPWdKLQiob5MK/LSFSqFDTdlFluq9qz1z7HO4DnpbxNhDCUdFxV+6OajRLF6w53j/xWQ0iQGAshtpIUcMN9d9CpuS6K716XtBPn70khaxLqSX0NA1SG/l7ttyQ/IjmhdB133z8owgkIaCtug65beoMdoNmpBV72RAXR4GUbQKEcvUYhYzjUN4dK8bk2SWHe3JZQxbs5Dw9GAyElgKURtBqkRJSGuakbR2x+XW19EH2qwkV3idrsSZfCAxvSDY4V7sZMNCa1FmjAsHH8SmArOlZETUMP5shQ20vf9U6pFcdHIZ/gOoxknAMrOMDFQpU7CvtjSRsBl/hy0hFuBDDff6RLb0HACsZwtRVhCkH/9HaXZG6Fz779tPAFfin3M1x14M7ITH5euAxuifx014IpJuiy2Zlcb8dcBaBou5Oq+CX2ZZ/UZYBMEsSfBKsrUkyXmjtvuU06KUhChodqZkurF/pPWrnFUcxii1Tm90UABzzWFp+YTCoe7k4XoCOtM0FwveRvWeENjrpw1j4G21Ph9hD/ziQnVLNTTaK4ElNaQCNCQ45+76SrLF7UwlVotBNTLqDvzV29sbu3u5BOxwEKocRoAK3B3R7+OugoMhrZ72uARX+O4U86xR9JY7OB++YGm2I2nCiWM5fS3nJo2/ootSmQjBmut5rom+HjCzuNGDToMcFqd77huJlYUAqizZVHaagptib9ocNFr8PkA2FBWnkAjXqLjTvd8l1aeD/MngA+3mzLvdmsGsNubHL7lzYPxbkQ2OEnUrCHl6PrCslMSkXveeIU6+Mhqd/84bSnd2A2mEc+KTgz0EL8vr7TOmxxqGRmWGrdswH4bL7mbS9zQ65PaIagidEH0X/UwBrbKQvR+lF3Gs+rmfdpFDkBZj+keR3lSwQ0/JlD6QNjQ+31LvsY+KY8vPT8C8LBDnrfNN9dZoNa6gz3WtT0DfXPla4ECJ2NnpQ3q NUpuxOyc o0rbzrYXFVGRQhH8uSdwwMaJLI5W7bSlddWbo/241WXRAmm0R5ydNQvLEzFTZSNSbt49+dK6cLW5OIEiFe5HoKMtcj6Pyo4Xfr2f/yEbMMbtubu0OcK9IYVp/QpCwVKLGv0gYlx0HDRjMa/JrfDXbFPXHwc+7V7Et2RKXcN40HId68TfXfwoAkm+tUjCNXWlJPIJdSMfdGnoQ+HLhDrBjdflsp99HJubdSRahaHuUKADrI2y9Ih9L8I+mmiUjkflIZpUSYGrZ4ITb6/Q= 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 17/07/2024 16:12, Pankaj Raghav (Samsung) wrote: >>>> >>>> This is really too much. It's something that will never happen. Just >>>> delete the message. >>>> >>>>> + if (max > MAX_PAGECACHE_ORDER) { >>>>> + VM_WARN_ONCE(1, >>>>> + "max order > MAX_PAGECACHE_ORDER. Setting max_order to MAX_PAGECACHE_ORDER"); >>>>> + max = MAX_PAGECACHE_ORDER; >>>> >>>> Absolutely not. If the filesystem declares it can support a block size >>>> of 4TB, then good for it. We just silently clamp it. >>> >>> Hmm, but you raised the point about clamping in the previous patches[1] >>> after Ryan pointed out that we should not silently clamp the order. >>> >>> ``` >>>> It seems strange to silently clamp these? Presumably for the bs>ps usecase, >>>> whatever values are passed in are a hard requirement? So wouldn't want them to >>>> be silently reduced. (Especially given the recent change to reduce the size of >>>> MAX_PAGECACHE_ORDER to less then PMD size in some cases). >>> >>> Hm, yes. We should probably make this return an errno. Including >>> returning an errno for !IS_ENABLED() and min > 0. >>> ``` >>> >>> It was not clear from the conversation in the previous patches that we >>> decided to just clamp the order (like it was done before). >>> >>> So let's just stick with how it was done before where we clamp the >>> values if min and max > MAX_PAGECACHE_ORDER? >>> >>> [1] https://lore.kernel.org/linux-fsdevel/Zoa9rQbEUam467-q@casper.infradead.org/ >> >> The way I see it, there are 2 approaches we could take: >> >> 1. Implement mapping_max_folio_size_supported(), write a headerdoc for >> mapping_set_folio_order_range() that says min must be lte max, max must be lte >> mapping_max_folio_size_supported(). Then emit VM_WARN() in >> mapping_set_folio_order_range() if the constraints are violated, and clamp to >> make it safe (from page cache's perspective). The VM_WARN()s can just be inline > > Inlining with the `if` is not possible since: > 91241681c62a ("include/linux/mmdebug.h: make VM_WARN* non-rvals") Ahh my bad. Could use WARN_ON()? > >> in the if statements to keep them clean. The FS is responsible for checking >> mapping_max_folio_size_supported() and ensuring min and max meet requirements. > > This is sort of what is done here but IIUC willy's reply to the patch, > he prefers silent clamping over having WARNINGS. I think because we check > the constraints during the mount time, so it should be safe to call > this I guess? I don't want to put words in his mouth, but I thought he was complaining about the verbosity of the warnings, not their presence. > >> >> 2. Return an error from mapping_set_folio_order_range() (and the other functions >> that set min/max). No need for warning. No state changed if error is returned. >> FS can emit warning on error if it wants. > > I think Chinner was not happy with this approach because this is done > per inode and basically we would just shutdown the filesystem in the > first inode allocation instead of refusing the mount as we know about > the MAX_PAGECACHE_ORDER even during the mount phase anyway. Ahh that makes sense. Understood. > > -- > Pankaj