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 CD8D0C3DA42 for ; Wed, 17 Jul 2024 09:59:37 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5AF6F6B009B; Wed, 17 Jul 2024 05:59:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 55FB06B009C; Wed, 17 Jul 2024 05:59:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 426BF6B009D; Wed, 17 Jul 2024 05:59:37 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 23DA36B009B for ; Wed, 17 Jul 2024 05:59:37 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id BBB68A18C5 for ; Wed, 17 Jul 2024 09:59:36 +0000 (UTC) X-FDA: 82348797552.23.8CA4AA5 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf08.hostedemail.com (Postfix) with ESMTP id D983C160021 for ; Wed, 17 Jul 2024 09:59:34 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=none; spf=pass (imf08.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=1721210356; 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=hEvgYMns+w6uuwYQDlgXgLMi8PYpbyPpzNOUpTp9ZxA=; b=bO/uedPDLoqsdn3ubwhqXhgTEynTOatjYRFTGhpvlqjsTa0FSIAfRIW60en3QGqfL80XW5 5U/oDnDo9ZzYVDPEtTGckhV4Z6mFMDp/IYeeR4pX5J447sTqRsenwe54xhf1nPWnqzhrSN DjTm9WYClKr9lWYmJ7QgcPGFRBX1kGU= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=none; spf=pass (imf08.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=1721210356; a=rsa-sha256; cv=none; b=8L2KSarP15vWdS4+kg4iiAvwQFdPkaibHUGd6d7DNnazTWjLUj2VdJrDOCtXbWMi8NnYIN 2DheX7NkAt3mCnEsilvGkApNEz6YEIIPn2jD6e/vty0Lb2DLyVBoeEkXvMPicZs1AF6uSJ RZBK7Z4zP2btKlBy1gLKE7IcpRNHM2U= 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 4301C1063; Wed, 17 Jul 2024 02:59:59 -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 D19213F73F; Wed, 17 Jul 2024 02:59:29 -0700 (PDT) Message-ID: <61806152-3450-4a4f-b81f-acc6c6aeed29@arm.com> Date: Wed, 17 Jul 2024 10:59:27 +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)" , Matthew Wilcox Cc: 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> From: Ryan Roberts In-Reply-To: <20240717094621.fdobfk7coyirg5e5@quentin> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: D983C160021 X-Stat-Signature: mou4kwr4srtz97isse78terat7tn6pyo X-HE-Tag: 1721210374-303484 X-HE-Meta: U2FsdGVkX19qMixVhz2PuxHwx5AI7rr0tbap+J7qLNmRk1/TWD7WAAjjICwKtmlovosvXlBQYt6j6gRlSxIbE2wsHsWeuEII/h+rlvpR+uf31cJ4mFkW3zSQs+zxEqfGK/MLtQJc6GDFjZEZstSA/usdNP8kh3lHpUwyqe1wXr0IY1DkdHuqfcLeaRS2XrGj0UB0Xqb855xnLQDFXH9vjycMIvRg6mRv4WWa/7RhYShBRpWM1KViGaU9i47cu720kMHfoJwRny0k2lyNKfidWk/Mc1DJtzebx9Z7Gyt49LYGD8md7ciCGOF3/W9NeYrHnVWmFgCGfMjkkSwGerLzlLgakxtxaE830FJ/I3Dgo8QMeVBup+8RD5xBpgUoA4bLd3i7HRhttzxnTHoThXGCZVDbnNhh4p8p2aqhC9NNv9W6G/VKV7mgzNyDTLWQX7M5coPrWKTBasIvCXkQ4fEdiw02t0ZoY4M/kbTMRHDECE6DAdu0KRhFjnAn0tSV1rI+FSMWaLNsCEVQlU2w34Dc7bySq86jzPA3Pu26fcGJPLTB4/AS2kERteutLlZ2jggg1vHWoNWudr3plgLlB+YZQf6ogBZw2T/WhXe+XXKwc9oIR7dFYAaS+10GDFg75GFwSJ3AjMr8i5nBgFL0/CJXFTlI90DFxeCuxGd44RHvDytL2QLtJLUoKi/VH+xCho52KNupzAUGVnnZQqCUv0YJMIUfEch3PmGXCYJbU8c4OEoe4bOICJKF5tcTB662Hm1OX5y2Q7xLtHsGr56si7IyKqvBCgrx1ZM4e0RotBKyRK+d4NNAfJ3X2+HS8UU1coW3YUb5NNUjXdxNzoQUJ6Fi8lp9rat46LRW9zVUOtkU7ejuYsN5S8fZhQus4uj9WM8GHwfyoQ5V6lMfdvxHH+950XASNMxkV2/Q20DUmaCE5Ge5axppAd9dppH+/RsVEMjdnRDc6i1/6M37xx65spD /MbqrDvw UY+Wy29GZRZhJwpdlOZ1EFIN252KgUoKoARMRGQxYbSoSyDi77iJ63nFtUz94nQCdtIMbi5d2o/o0kVKBtUwM00zLUvh92juU2Sq4hYT3vS+abxP7Bpv2AlizQXxE4A1TcPCEJsQi66D4/5+DTrP0UHjNBz5M+KnhP5X+2DVKnZqk9Y7N88n21xBzWwBfrvZYTYccDZOUevY6mhptZYgXnWbz3HrQkOzprTH4gY6rc8BAoAeHjWveYtCUWcJ1NG9eLmcp+xluvOGB9Yg= 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 10:46, Pankaj Raghav (Samsung) wrote: > On Tue, Jul 16, 2024 at 04:26:10PM +0100, Matthew Wilcox wrote: >> On Mon, Jul 15, 2024 at 11:44:48AM +0200, Pankaj Raghav (Samsung) wrote: >>> +/* >>> + * mapping_max_folio_size_supported() - Check the max folio size supported >>> + * >>> + * The filesystem should call this function at mount time if there is a >>> + * requirement on the folio mapping size in the page cache. >>> + */ >>> +static inline size_t mapping_max_folio_size_supported(void) >>> +{ >>> + if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) >>> + return 1U << (PAGE_SHIFT + MAX_PAGECACHE_ORDER); >>> + return PAGE_SIZE; >>> +} >> >> There's no need for this to be part of this patch. I've removed stuff >> from this patch before that's not needed, please stop adding unnecessary >> functions. This would logically be part of patch 10. > > That makes sense. I will move it to the last patch. > >> >>> +static inline void mapping_set_folio_order_range(struct address_space *mapping, >>> + unsigned int min, >>> + unsigned int max) >>> +{ >>> + if (!IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE)) >>> + return; >>> + >>> + if (min > MAX_PAGECACHE_ORDER) { >>> + VM_WARN_ONCE(1, >>> + "min order > MAX_PAGECACHE_ORDER. Setting min_order to MAX_PAGECACHE_ORDER"); >>> + min = MAX_PAGECACHE_ORDER; >>> + } >> >> 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 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. 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. Personally I prefer option 2, but 1 is definitely less churn. Thanks, Ryan