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 CAEFDC54E41 for ; Mon, 26 Feb 2024 14:47:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1ABF5440173; Mon, 26 Feb 2024 09:47:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 15AC0440147; Mon, 26 Feb 2024 09:47:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 02381440173; Mon, 26 Feb 2024 09:47:42 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id DDF73440147 for ; Mon, 26 Feb 2024 09:47:42 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id AFFFFA0C8F for ; Mon, 26 Feb 2024 14:47:42 +0000 (UTC) X-FDA: 81834233964.19.12CAEDD Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf21.hostedemail.com (Postfix) with ESMTP id 3EE661C0008 for ; Mon, 26 Feb 2024 14:47:41 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="L/tRy2rX"; dmarc=none; spf=none (imf21.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1708958861; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=yza63Q+E0MQPCYS+BMi7MSICGP+bYwaIfeMJZhSmHAE=; b=fP+iquGu/9LPoT3dXuOG/qJdfoBuJY680zlfoyjerS3zXAo5vJiJSAxTpCePMPMkPeSh3V bjIEXAj++RySe53UIRsyAk+Ph63qfN6+npbJ45wiUjniPg2sJeFQlzYWxJiLcJR7Ta7wRm OgFWXxLIyT38PMObuk52lvnEs1zHNFU= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="L/tRy2rX"; dmarc=none; spf=none (imf21.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1708958861; a=rsa-sha256; cv=none; b=04QMrZ8Ax4tqJEWTKzGHNznsl719Xcg1o2a4BalCIRh+q/DSz59TiqkAlZG/Tgm+FvAnLS XgFm3oUaQVoUrRjLn03FIJ7GAEA1Dna3aQx6z230o2JO9tFfEAgjH7TYj4CJ0wCTSPDdyD hYF/1z7q22WadrQvgIgwGYoywmMp2Lo= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=yza63Q+E0MQPCYS+BMi7MSICGP+bYwaIfeMJZhSmHAE=; b=L/tRy2rXVkixtFmv5H9i8Qe5nM Rkutf9gUoZz55MoYgGdoS2SLSelhnx5JIy6iVW9qOmBQwDO97pFD0E6gFMz+j/5zakLyUBbdjxKtd 0f7tR03m2a/31EnhM9wx122E7iYxVtO6DqesPtXlMfvsd8drPWY3a4GhY+ZUUkupNqwJ7J9zD/uQj YGUkzB+jpyB8gLppfcIacfTh/cULtqx5UXA1wS4Wflt/BC0T/wGbFoJVtBPLINgK4+qVXv8nGt0l3 Dpl4cGVzMdnG56AI4qqdd6rbuQpsRIGVFLF5Gs9uCTcDY3MN93UQoHIiiR/q8JRYAJqUiZ942gv8q ELe8gzUw==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1recGT-0000000HQ83-33XC; Mon, 26 Feb 2024 14:47:33 +0000 Date: Mon, 26 Feb 2024 14:47:33 +0000 From: Matthew Wilcox To: "Pankaj Raghav (Samsung)" Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, david@fromorbit.com, chandan.babu@oracle.com, akpm@linux-foundation.org, mcgrof@kernel.org, ziy@nvidia.com, hare@suse.de, djwong@kernel.org, gost.dev@samsung.com, linux-mm@kvack.org, Pankaj Raghav Subject: Re: [PATCH 04/13] filemap: use mapping_min_order while allocating folios Message-ID: References: <20240226094936.2677493-1-kernel@pankajraghav.com> <20240226094936.2677493-5-kernel@pankajraghav.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240226094936.2677493-5-kernel@pankajraghav.com> X-Rspamd-Queue-Id: 3EE661C0008 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: esdhaa5t4x5o45wtbcadb3xareycj8sq X-HE-Tag: 1708958861-645140 X-HE-Meta: U2FsdGVkX19NxovDb/K8iYb57KjStHyHUsDksm6OoH9Sj7m++m3ZCx6jD1YnfxXHOXNeEDTfR/0g1iRu+TqpohFELm2mTSZzgucn0zwpu/y3lWfe+xB6L3jUA9WtDIyC/D6oG9A+KghMW/+qcorb/HlxbjFY80fzj9SYt9dq0egjzMLMWFxyTcXBLhuqhAxN+/ZHc0Zsin5NUT1FUl8soH7WNu64Um2OU+eLPqdfXt4jUpZoBUCRAQCxovbkT0HtWbUvhzXfK/8/otlaXWjuzV/sj9IrsCXtpoav6n6K422bQMsVZgjDCAmX1UIjnxDsIcXFL0Pjg/u9E+pyWwolxD+QRMs3lPODHouO4ydpVIwX0TkK3lfnkBMnClqDn+Co5OhNcXA69DPI7SVWXwTbyP4wTrnn8iWBLXlSEyMzsUfCMf/NyzOoU3mmdov00Sc/uE1sgulvYQFY3HgP1m1C4v5HFPRrSKSHgKooHH5cgRYE68melb6/0rE0Iqr7P7Eba+5tDliie1fvKmREtEWqN3fqADFu7thIuHSMQelWuqHRqUDvcPOvnSLAjozSWjtYlILHtfvMNwhP5Br//qdGQlBPZ8GSx7lbHLFYgz6KbKhklDOjWzenKLjLv1PLiqnnkL2oQ7+KZbSGeWKUK0NVoFfULODxCg4lmy1v8GJZO7kq0nQNBCwyUC1kim/D2mTx80IBpuo1Rhm6GbevMcaV8FBsY3P61+2CK4/AA1KSuYHGdDqkPwj4kwsO924EMpVz8U5QtwAND3EqeJI6DecO0hyzBok5KpJ02sDvVhQpkXYBx6E8ee3sFBbyZBSxlVT8vqxUz1FKWXns1WyXp3eB1wkz7nuXLK8E67bs3DICfN9ekSs2TtRaHFdwxf8vSWG8 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 Mon, Feb 26, 2024 at 10:49:27AM +0100, Pankaj Raghav (Samsung) wrote: > Add some additional VM_BUG_ON() in page_cache_delete[batch] and > __filemap_add_folio to catch errors where we delete or add folios that > has order less than min_order. I don't understand why we need these checks in the deletion path. The add path, yes, absolutely. But the delete path? > @@ -896,6 +900,8 @@ noinline int __filemap_add_folio(struct address_space *mapping, > } > } > > + VM_BUG_ON_FOLIO(folio_order(folio) < mapping_min_folio_order(mapping), > + folio); But I don't understand why you put it here, while we're holding the xa_lock. That seems designed to cause maximum disruption. Why not put it at the beginning of the function with all the other VM_BUG_ON_FOLIO? > @@ -1847,6 +1853,9 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > fgf_t fgp_flags, gfp_t gfp) > { > struct folio *folio; > + unsigned int min_order = mapping_min_folio_order(mapping); > + > + index = mapping_align_start_index(mapping, index); I would not do this here. > repeat: > folio = filemap_get_entry(mapping, index); > @@ -1886,7 +1895,7 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > folio_wait_stable(folio); > no_page: > if (!folio && (fgp_flags & FGP_CREAT)) { > - unsigned order = FGF_GET_ORDER(fgp_flags); > + unsigned int order = max(min_order, FGF_GET_ORDER(fgp_flags)); > int err; Put it here instead. > if ((fgp_flags & FGP_WRITE) && mapping_can_writeback(mapping)) > @@ -1912,8 +1921,13 @@ struct folio *__filemap_get_folio(struct address_space *mapping, pgoff_t index, > gfp_t alloc_gfp = gfp; > > err = -ENOMEM; > + if (order < min_order) > + order = min_order; > if (order > 0) > alloc_gfp |= __GFP_NORETRY | __GFP_NOWARN; > + > + VM_BUG_ON(index & ((1UL << order) - 1)); Then you don't need this BUG_ON because it's obvious you just did it. And the one in filemap_add_folio() would catch it anyway.