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 6F1BCE7717F for ; Tue, 10 Dec 2024 11:31:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B6B3D6B0188; Tue, 10 Dec 2024 06:31:34 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B1AE76B0189; Tue, 10 Dec 2024 06:31:34 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E4366B018A; Tue, 10 Dec 2024 06:31:34 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7A5FB6B0188 for ; Tue, 10 Dec 2024 06:31:34 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 21181120843 for ; Tue, 10 Dec 2024 11:31:34 +0000 (UTC) X-FDA: 82878833478.12.A05ACE5 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf22.hostedemail.com (Postfix) with ESMTP id 1622CC000A for ; Tue, 10 Dec 2024 11:31:08 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=MLfRIkxm; spf=none (imf22.hostedemail.com: domain of BATV+8c38dece5ebcc54df4cc+7779+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+8c38dece5ebcc54df4cc+7779+infradead.org+hch@bombadil.srs.infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1733830277; 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=Feb0jQOujGmxXR8PPkTXLVIfG9wGJNk4iUPyw0CUCJk=; b=RuFcE26Qy9sAYvPXPOm9yR7JXdCjo9x3whe7yyzq2dldBrm5BglWAJS9jcLlF1gEvhu4le No/GhU5Jm1MyMaiTba4OJFONl/DW6Z38LNR10pNDXHzqGWLnrW7V+enxLzKCTFssimWB5f lP44YRBRzItlJDj+9YfTDyPbYTjY/Bc= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=MLfRIkxm; spf=none (imf22.hostedemail.com: domain of BATV+8c38dece5ebcc54df4cc+7779+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+8c38dece5ebcc54df4cc+7779+infradead.org+hch@bombadil.srs.infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1733830277; a=rsa-sha256; cv=none; b=vsKw+VYKHmZmbpldOGelBnbcy6YSsb8UnDf1hUKQwkKWKgKZ5rfToKrg82XGVOwzAQf/3B S/u0ZKjeIg0GbLJaI534sizaJK7Pg6V7N5U3uHLBPbXQ6nade+b96jOwuMjCV4k8cK5vF8 FOEZdWexcTiiT/CJZFHhbJK2Rc+Z1t0= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; 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=Feb0jQOujGmxXR8PPkTXLVIfG9wGJNk4iUPyw0CUCJk=; b=MLfRIkxmC678SS76Jwg2qowzDG MVcEEk5oKwIV+Zo3/Xi1v8CIsS0OSNiWMxO90sJsOhj/PgH+nFLVcDPHx8BkD7a0X+BZRBPhFf/kh 6wTmJzYom4GQC1UYADZmKHT42oXD+zVpiqC8EJpt5yKqGRZDNlBp625Hlt8Vupv7bY8GJUpVz2zvt OTXh8nxAvd2MzmIrCslZ6eyW3G3l/EMwZrPf+6+Ocq0FkkEXOHAGt0WynxEI1YebHD4wBFX6NooS0 iYR9kFFGZ7ayxwI8ctp0my/fftrO6xL+nT3YZfckqJZhqNKoRNJ2xawBzdGrPiydCi0g/wXw2crLi 6oTukqmQ==; Received: from hch by bombadil.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tKySh-0000000BJy9-0FwJ; Tue, 10 Dec 2024 11:31:31 +0000 Date: Tue, 10 Dec 2024 03:31:31 -0800 From: Christoph Hellwig To: Jens Axboe Cc: "Darrick J. Wong" , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, hannes@cmpxchg.org, clm@meta.com, linux-kernel@vger.kernel.org, willy@infradead.org, kirill@shutemov.name, bfoster@redhat.com Subject: Re: [PATCH 11/12] mm/filemap: make buffered writes work with RWF_UNCACHED Message-ID: References: <20241203153232.92224-2-axboe@kernel.dk> <20241203153232.92224-13-axboe@kernel.dk> <20241206171740.GD7820@frogsfrogsfrogs> <39033717-2f6b-47ca-8288-3e9375d957cb@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <39033717-2f6b-47ca-8288-3e9375d957cb@kernel.dk> X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-Rspamd-Server: rspam05 X-Stat-Signature: 8ae7uc7g7r19x3zqdfrha54pywss9dgj X-Rspamd-Queue-Id: 1622CC000A X-Rspam-User: X-HE-Tag: 1733830268-40082 X-HE-Meta: U2FsdGVkX19GmWeav166t4YGQwSlvVOJ6yW3axBaqxHP6/kOR28wjlkUNF2fDKY9bZTQwhK+CUgdUbykLbnv2KnvL7D0Af1uHsxW4W7ox+gj1bjQ9WDOcJoEO/7M+KpKfFzVQFXfnJ4yLc460/+9WPjUZLSzc3x6+HH9RkznuLpMjeJtx5BsXF1uvbSsijmwzbVWPjXPbUp/fyD1JRlu/HS3AwP2lD2Da91yzbvClhGsD2HyGKgZqqoWPYa/lxqA3NTOspqukWzv6aD0pc0GzbIbhZ4/V3FfnWiOeuPqfcC+7YOha3ZhnIucmWtWIaswWZvP9NkQbksv11zNqMw9aDzMIdPmGZJrViZyqRIOCBzoviZEPDUnVlz01jiOTIYgEQS+hj6IAhPR4Ygpj45QlLqIobvqKZgZPAmDF3wRSMj4mhmK6suDeYSMMSI3buWSqzHU/30boJrjsLuIX2J+cSyUNDGgc4pHUZoePmFbRksEg+2y06tOaSBSKhHEivHk0oJ/8qTmn3kzvJx7PBYcaxq+WRTueIW9AQEB/QOI3nfld8rTrUqazC8NZ89UITFxUHtkIFvIoKor47p3kgkKPCJq01Q7uAk/pvBTZtU9MxjkqGKsTZy/7EtFZ4nc0gmjGIC8Zw9LixaKa/jGsu8fQjwnUFgredmgRB0usskk4J+0fnyIl/AhvFvWzJmuCGou5j1L4h6FKSRcVW/XQWUkabMKIu1qmYgJEjzEr0ZpXnb0TWTnAEbemXZxWjrlMb/tbxqir32FMdaGcIMqczKcRxFPfnzzpd++rhpFys+K3jT1gz5E0VF2uIowytcpY17GxrWdKjAX5btiPEDk1ZzpxTecaeNAJhDRlHAQM9lVmmvZZDoSI3rL0zyiexWvKZ0Ho2DYgEF2B2RaalYnTV292RPeKVpEhfnHGzkvX3AIX0GYFY+CLwPwG514azZRkz29kFEk9BLWiwY0vHpGbbd nhf5hCEo yKPa1rdVjC2x39bBmSGRtpkH4UsMadGqZvMm4KrYPMnFnO/Plri2A6Op3VgCvzY32UIDvp1ZO5Dx5/WVBwlxpHL37zCfSfNqTvUF/5Ph/kxTYd7aZPxCG2MWevuQtIt0mDC8cFhGqqRLPrCJzqk1HWirLK04T6F6QwaGPOl0W1rS67J051it4ThuneCK+BRRH5/Nq9Sqn5stm7vCIG+zPGlcUfstLYLcWrNfCOmvVJ82oxALRSvcHi5aunDb/7OTnUSNxERczfQwUfAck9rDY+Q3/uuWh8jT/7kEbKrY6lSPGJimRr55/g6q2MkGS0Rb3foKssMkYCizrrg6ghHAss5wUTWHKdTqm4FdNQ9TbYNGkQpGouovi6arPBnKLIL9/d0rDaeL/jRsvBQ8mjyMcGKb6jBdGARfyhq7DyCSk8W2iRUg5gCW/u+BUEfB+o8kpdL5Q7UY4OhjaNGOYJqgDM/O9W8e07ZC1dnvPWMbEFdAY673gzYCaJAcg4LaDYDjd1v4WTVpGai8OLriJAXESUKsOkw== 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 Fri, Dec 06, 2024 at 11:22:55AM -0700, Jens Axboe wrote: > > Honestly, I'm not a fan of foliop_uncached or foliop_is_uncached. > > It definitely is what I would elegantly refer to as somewhat of a > hack... But it's not _that_ bad imho. It's pretty horrible actually. > > I think these two macros are only used for ext4 (or really, !iomap) > > support, right? And that's only to avoid messing with ->write_begin? > > Indeed, ideally we'd change ->write_begin() instead. And that probably > should still be done, I just did not want to deal with that nightmare in > terms of managing the patchset. And honestly I think it'd be OK to defer > that part until ->write_begin() needs to be changed for other reasons, > it's a lot of churn just for this particular thing and dealing with the > magic pointer value (at least to me) is liveable. ->write_begin() really should just go away, it is a horrible interface. Note that in that past it actually had a flags argument, but that got killed a while ago. > > What if you dropped ext4 support instead? :D > > Hah, yes obviously that'd be a solution, then I'd need to drop btrfs as > well. And I would kind of prefer not doing that ;-) Btrfs doesn't need it. In fact the code would be cleaner and do less work with out, see the patch below. And for ext4 there already is an iomap conversion patch series on the list that just needs more review, so skipping it here and growing the uncached support through that sounds sensible. diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 444bee219b4e..db3a3c7ecf77 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -894,20 +894,17 @@ static gfp_t get_prepare_gfp_flags(struct inode *inode, bool nowait) */ static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ret, loff_t pos, size_t write_bytes, - bool force_uptodate, bool nowait) + bool force_uptodate, fgf_t fgp_flags) { unsigned long index = pos >> PAGE_SHIFT; - gfp_t mask = get_prepare_gfp_flags(inode, nowait); - fgf_t fgp_flags = (nowait ? FGP_WRITEBEGIN | FGP_NOWAIT : FGP_WRITEBEGIN); + gfp_t mask = get_prepare_gfp_flags(inode, fgp_flags & FGP_NOWAIT); struct folio *folio; int ret = 0; - if (foliop_is_uncached(folio_ret)) - fgp_flags |= FGP_UNCACHED; again: folio = __filemap_get_folio(inode->i_mapping, index, fgp_flags, mask); if (IS_ERR(folio)) { - if (nowait) + if (fgp_flags & FGP_NOWAIT) ret = -EAGAIN; else ret = PTR_ERR(folio); @@ -925,7 +922,7 @@ static noinline int prepare_one_folio(struct inode *inode, struct folio **folio_ if (ret) { /* The folio is already unlocked. */ folio_put(folio); - if (!nowait && ret == -EAGAIN) { + if (!(fgp_flags & FGP_NOWAIT) && ret == -EAGAIN) { ret = 0; goto again; } @@ -1135,9 +1132,15 @@ ssize_t btrfs_buffered_write(struct kiocb *iocb, struct iov_iter *i) const bool nowait = (iocb->ki_flags & IOCB_NOWAIT); unsigned int bdp_flags = (nowait ? BDP_ASYNC : 0); bool only_release_metadata = false; + fgf_t fgp_flags = FGP_WRITEBEGIN; - if (nowait) + if (nowait) { ilock_flags |= BTRFS_ILOCK_TRY; + fgp_flags |= FGP_NOWAIT; + } + + if (iocb->ki_flags & IOCB_UNCACHED) + fgp_flags |= FGP_UNCACHED; ret = btrfs_inode_lock(BTRFS_I(inode), ilock_flags); if (ret < 0) @@ -1232,11 +1235,8 @@ ssize_t btrfs_buffered_write(struct kiocb *iocb, struct iov_iter *i) break; } - if (iocb->ki_flags & IOCB_UNCACHED) - folio = foliop_uncached; - ret = prepare_one_folio(inode, &folio, pos, write_bytes, - force_page_uptodate, false); + force_page_uptodate, fgp_flags); if (ret) { btrfs_delalloc_release_extents(BTRFS_I(inode), reserve_bytes);