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 1A9C9C61CE8 for ; Mon, 9 Jun 2025 16:04:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8EDA86B0088; Mon, 9 Jun 2025 12:04:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 89DAA6B0089; Mon, 9 Jun 2025 12:04:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 78C9A6B0092; Mon, 9 Jun 2025 12:04:24 -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 55FCC6B0088 for ; Mon, 9 Jun 2025 12:04:24 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id C5882BC7C4 for ; Mon, 9 Jun 2025 16:04:23 +0000 (UTC) X-FDA: 83536334406.27.768C355 Received: from nyc.source.kernel.org (nyc.source.kernel.org [147.75.193.91]) by imf14.hostedemail.com (Postfix) with ESMTP id 23C73100002 for ; Mon, 9 Jun 2025 16:04:21 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R7gNVkhh; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf14.hostedemail.com: domain of djwong@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749485062; a=rsa-sha256; cv=none; b=IfR745O8Lv0+qxbvZzxnYUuy3khHqTLZUkwonb7ST1nPMRpYMtFyocqCislxpOxMFykcfw 6gyOJy7Siu58JU/uw5fj6h3ToWZqgyfJE47Y9z9eDdhG04IuB8DrL4xBhT3Ga6v/OohpHd 9sXhSdwrKv0KpfXXe+CRj3niGZPbWB4= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R7gNVkhh; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf14.hostedemail.com: domain of djwong@kernel.org designates 147.75.193.91 as permitted sender) smtp.mailfrom=djwong@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749485062; 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=0WlI7QbH7rNM1wL8r0H7Gt2qPpjcrbl1eaDvrNQAm0Q=; b=Ch+VGMLTT4fkq0lvjbFFvab4Dz1oYIM7mA3gHIXGtOroRcXfWr2KVjs9PWKOtsZBgLGa0E WaUsoIF7KyiWp4b7JY5EAn8AGTVrk5HFmUihvcaSLTfjpoM/qdLtTSzI3DtEuXAbT713N2 hR61wX2u3rCoVq9tmDAQUhbiucyPBlw= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by nyc.source.kernel.org (Postfix) with ESMTP id 51839A4F736; Mon, 9 Jun 2025 16:04:21 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EA9BCC4CEEB; Mon, 9 Jun 2025 16:04:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1749485061; bh=kHl0G3xtQFEcPUPWFdl2N0Jc4xNPDnnPZA4BjNK7xlk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=R7gNVkhhAW3qKsqea1AMXf8XUOrEqGGIEcctVmN7Zc3Ud/LKOujO8g5v0HeZIklPo us2o09MXFGJghg133gMQGsa4maGTN5fDN252bW+qGav7Rf1FnJHO/lRmjWAsIXII0p Ym+mJ8bhZxgw7aWeRAzcj8D9BOVfy8iG1I9u3VleGtN6aMPsCORAI6bUmaR5Q2dfUp w/qg1ErwroYfhCkWRxfJxrXHpgRiGwhzHnlOZnlFuJ9fqlykaGmKGJZdlsUiv//zCb fCO0EX12ey3UFDLNmUUbb+xwhJFMR73XhLMNKuhTBXq5TnVbbyQCuRLEFIzSZCxUsw sR6vDWLKk6yiQ== Date: Mon, 9 Jun 2025 09:04:20 -0700 From: "Darrick J. Wong" To: Brian Foster Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 3/7] iomap: optional zero range dirty folio processing Message-ID: <20250609160420.GC6156@frogsfrogsfrogs> References: <20250605173357.579720-1-bfoster@redhat.com> <20250605173357.579720-4-bfoster@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250605173357.579720-4-bfoster@redhat.com> X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 23C73100002 X-Stat-Signature: 93oadwaihhw99b1mqyte6duex1fb48be X-Rspam-User: X-HE-Tag: 1749485061-790808 X-HE-Meta: U2FsdGVkX190QSwL8w0SmjXp1F5xMBZgf4snnWuqBFnX6aCpBeq+TyKPNATAdlH/AkxyzJrLdfodVBu9G9AKBT17WJASFP2u96NK3N7upqZ/MXJIkx7fGGmBtqLHLMRI/7gW7ngY8299E6LcJjWnU43BtIsXapBHIsstsnbSdUj3FJwB9gq3sO8Y3s+2lLnf+L9P4d0JJ/KrdtTKA3WWNw1EIsMIQY5UPGlEXiLJZb3ZPbWhzjqMfpIrDoUSAXfENLBcjCO0Ax8Qb0XUIwfBhlOdcLO+pVRA0/13ujNHyKK1eGzKb2dHaAW3NSCY0V2cP02ytJkQQYFZKmekxzdewxiuJYHnrL/I8FZ6vKsdFqMR4QhLYj+iXAO4sfRekJGEMGrd4CegPfkvnZx5ru/oAyqe9BHGzkUinSWyqJltq+9iVyxbQzkb4itogRl3f2GhU/0mtvSCKTJCJH+mHVpj3qh1xifH0IinJ2KsSlXfKpVgVSm0u/lgOXkX3isEXoQNfrojeAS4HlD60ATl96oUpqAKPjQRWCKiFiHxxSXR0xswR73ngPzF6RBzpX8+XRW9vbVRHpj0aGJPVXpThQXxI8F/coxkKbaVyHFQtQrXskN7oDnTWQUjRyeCqA3Ch5LAjzCpOfypMr08lj5AL7ABjGyu1kdP2pMvWXMFd/MCKYfVPVBB8lEj8Bf61qzgIPBSfSKoiwrZbCI2vacpH2LiHnQOY+cZWSX0YzLCJBwk//KYR+2XAjJJqV79bZJ6dKSyLyGAwqsHXP8BVN3v8nMSe9TBN6Nmo42ifhkIL+8py2sUmbG5gSlHGVTE/ul1hWu+IbOpqVuXjt60S8QpMN+mBIuRNngvQ4RXWWfKUyTh2XfY0iEzuACKxn4tvGES4BN/CWhomG+LSnRUmRNVx3xukCDF50vcQWozg3nTvAf0KNV51WoK9fpi53tTkHp50lbZ+R1xpbLcOREKWN4fssT FIHiQGUg eXUvpZ7+zAYpFIN17ONYPM5YVucZpIs/6Q+bRv6nLCVC+2LnTSawp1eTlWoNMso1hx1bLAwpsrqrNAp06bsQi6wQsADJg2iVYW7kGq17TImF6he4xIKj4EKQbN91SJF8EEwjgmchtWlWDLclmBh+sBWCcw6y1OTm1a5VEqWeXt0phBxdx5zCEahcvGM5s1wJI80mRbtzsPxJ+8oVZYnyvOgMmHo25K4gshzxyUu+GWBxDuv7rYNF9Q7uHPaxG9WHlR84BtJ93gYGhO6LxiloyeWF/Ow== 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 Thu, Jun 05, 2025 at 01:33:53PM -0400, Brian Foster wrote: > The only way zero range can currently process unwritten mappings > with dirty pagecache is to check whether the range is dirty before > mapping lookup and then flush when at least one underlying mapping > is unwritten. This ordering is required to prevent iomap lookup from > racing with folio writeback and reclaim. > > Since zero range can skip ranges of unwritten mappings that are > clean in cache, this operation can be improved by allowing the > filesystem to provide a set of dirty folios that require zeroing. In > turn, rather than flush or iterate file offsets, zero range can > iterate on folios in the batch and advance over clean or uncached > ranges in between. > > Add a folio_batch in struct iomap and provide a helper for fs' to > populate the batch at lookup time. Update the folio lookup path to > return the next folio in the batch, if provided, and advance the > iter if the folio starts beyond the current offset. > > Signed-off-by: Brian Foster > --- > fs/iomap/buffered-io.c | 73 +++++++++++++++++++++++++++++++++++++++--- > fs/iomap/iter.c | 6 ++++ > include/linux/iomap.h | 4 +++ > 3 files changed, 78 insertions(+), 5 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 16499655e7b0..cf2f4f869920 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -750,6 +750,16 @@ static struct folio *__iomap_get_folio(struct iomap_iter *iter, size_t len) > if (!mapping_large_folio_support(iter->inode->i_mapping)) > len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos)); > > + if (iter->fbatch) { > + struct folio *folio = folio_batch_next(iter->fbatch); > + > + if (folio) { > + folio_get(folio); > + folio_lock(folio); Hrm. So each folio that is added to the batch isn't locked, nor does the batch (or iomap) hold a refcount on the folio until we get here. Do we have to re-check that folio->{mapping,index} match what iomap is trying to process? Or can we assume that nobody has removed the folio from the mapping? I'm wondering because __filemap_get_folio/filemap_get_entry seem to do all that for us. I think the folio_pos check below might cover some of that revalidation? > + } > + return folio; > + } > + > if (folio_ops && folio_ops->get_folio) > return folio_ops->get_folio(iter, pos, len); > else > @@ -811,6 +821,8 @@ static int iomap_write_begin(struct iomap_iter *iter, struct folio **foliop, > int status = 0; > > len = min_not_zero(len, *plen); > + *foliop = NULL; > + *plen = 0; > > if (fatal_signal_pending(current)) > return -EINTR; > @@ -819,6 +831,12 @@ static int iomap_write_begin(struct iomap_iter *iter, struct folio **foliop, > if (IS_ERR(folio)) > return PTR_ERR(folio); > > + /* no folio means we're done with a batch */ ...ran out of folios but *plen is nonzero, i.e. we still have range to cover? > + if (!folio) { > + WARN_ON_ONCE(!iter->fbatch); > + return 0; > + } > + > /* > * Now we have a locked folio, before we do anything with it we need to > * check that the iomap we have cached is not stale. The inode extent > @@ -839,6 +857,20 @@ static int iomap_write_begin(struct iomap_iter *iter, struct folio **foliop, > } > } > > + /* > + * folios in a batch may not be contiguous. If we've skipped forward, > + * advance the iter to the pos of the current folio. If the folio starts > + * beyond the end of the mapping, it may have been trimmed since the > + * lookup for whatever reason. Return a NULL folio to terminate the op. > + */ > + if (folio_pos(folio) > iter->pos) { > + len = min_t(u64, folio_pos(folio) - iter->pos, > + iomap_length(iter)); > + status = iomap_iter_advance(iter, &len); > + if (status || !len) > + goto out_unlock; > + } > + > pos = iomap_trim_folio_range(iter, folio, poffset, &len); > BUG_ON(pos + len > iter->iomap.offset + iter->iomap.length); > if (srcmap != &iter->iomap) > @@ -1380,6 +1412,12 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero) > if (iter->iomap.flags & IOMAP_F_STALE) > break; > > + /* a NULL folio means we're done with a folio batch */ > + if (!folio) { > + status = iomap_iter_advance_full(iter); > + break; > + } > + > /* warn about zeroing folios beyond eof that won't write back */ > WARN_ON_ONCE(folio_pos(folio) > iter->inode->i_size); > > @@ -1401,6 +1439,26 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero) > return status; > } > > +loff_t > +iomap_fill_dirty_folios( > + struct iomap_iter *iter, > + loff_t offset, > + loff_t length) > +{ > + struct address_space *mapping = iter->inode->i_mapping; > + pgoff_t start = offset >> PAGE_SHIFT; > + pgoff_t end = (offset + length - 1) >> PAGE_SHIFT; > + > + iter->fbatch = kmalloc(sizeof(struct folio_batch), GFP_KERNEL); > + if (!iter->fbatch) > + return offset + length; > + folio_batch_init(iter->fbatch); > + > + filemap_get_folios_dirty(mapping, &start, end, iter->fbatch); > + return (start << PAGE_SHIFT); > +} > +EXPORT_SYMBOL_GPL(iomap_fill_dirty_folios); Not used anywhere ... ? Oh, it's in the next patch. > + > int > iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero, > const struct iomap_ops *ops, void *private) > @@ -1429,7 +1487,7 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero, > * flushing on partial eof zeroing, special case it to zero the > * unaligned start portion if already dirty in pagecache. > */ > - if (off && > + if (!iter.fbatch && off && > filemap_range_needs_writeback(mapping, pos, pos + plen - 1)) { > iter.len = plen; > while ((ret = iomap_iter(&iter, ops)) > 0) > @@ -1445,13 +1503,18 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero, > * if dirty and the fs returns a mapping that might convert on > * writeback. > */ > - range_dirty = filemap_range_needs_writeback(inode->i_mapping, > - iter.pos, iter.pos + iter.len - 1); > + range_dirty = filemap_range_needs_writeback(mapping, iter.pos, > + iter.pos + iter.len - 1); > while ((ret = iomap_iter(&iter, ops)) > 0) { > const struct iomap *srcmap = iomap_iter_srcmap(&iter); > > - if (srcmap->type == IOMAP_HOLE || > - srcmap->type == IOMAP_UNWRITTEN) { > + if (WARN_ON_ONCE(iter.fbatch && > + srcmap->type != IOMAP_UNWRITTEN)) I wonder, are you planning to expand the folio batching to other buffered-io.c operations? Such that the iter.fbatch checks might some day go away? --D > + return -EIO; > + > + if (!iter.fbatch && > + (srcmap->type == IOMAP_HOLE || > + srcmap->type == IOMAP_UNWRITTEN)) { > s64 status; > > if (range_dirty) { > diff --git a/fs/iomap/iter.c b/fs/iomap/iter.c > index 6ffc6a7b9ba5..89bd5951a6fd 100644 > --- a/fs/iomap/iter.c > +++ b/fs/iomap/iter.c > @@ -9,6 +9,12 @@ > > static inline void iomap_iter_reset_iomap(struct iomap_iter *iter) > { > + if (iter->fbatch) { > + folio_batch_release(iter->fbatch); > + kfree(iter->fbatch); > + iter->fbatch = NULL; > + } > + > iter->status = 0; > memset(&iter->iomap, 0, sizeof(iter->iomap)); > memset(&iter->srcmap, 0, sizeof(iter->srcmap)); > diff --git a/include/linux/iomap.h b/include/linux/iomap.h > index 522644d62f30..0b9b460b2873 100644 > --- a/include/linux/iomap.h > +++ b/include/linux/iomap.h > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > > struct address_space; > struct fiemap_extent_info; > @@ -239,6 +240,7 @@ struct iomap_iter { > unsigned flags; > struct iomap iomap; > struct iomap srcmap; > + struct folio_batch *fbatch; > void *private; > }; > > @@ -345,6 +347,8 @@ void iomap_invalidate_folio(struct folio *folio, size_t offset, size_t len); > bool iomap_dirty_folio(struct address_space *mapping, struct folio *folio); > int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len, > const struct iomap_ops *ops); > +loff_t iomap_fill_dirty_folios(struct iomap_iter *iter, loff_t offset, > + loff_t length); > int iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, > bool *did_zero, const struct iomap_ops *ops, void *private); > int iomap_truncate_page(struct inode *inode, loff_t pos, bool *did_zero, > -- > 2.49.0 > >