Skip to content

Commit ed61378

Browse files
Brian Fosterbrauner
authored andcommitted
iomap: replace folio_batch allocation with stack allocation
Zhang Yi points out that the dynamic folio_batch allocation in iomap_fill_dirty_folios() is problematic for the ext4 on iomap work that is under development because it doesn't sufficiently handle the allocation failure case (by allowing a retry, for example). We've also seen lockdep (via syzbot) complain recently about the scope of the allocation. The dynamic allocation was initially added for simplicity and to help indicate whether the batch was used or not by the calling fs. To address these issues, put the batch on the stack of iomap_zero_range() and use a flag to control whether the batch should be used in the iomap folio lookup path. This keeps things simple and eliminates allocation issues with lockdep and for ext4 on iomap. While here, also clean up the fill helper signature to be more consistent with the underlying filemap helper. Pass through the return value of the filemap helper (folio count) and update the lookup offset via an out param. Fixes: 395ed1e ("iomap: optional zero range dirty folio processing") Signed-off-by: Brian Foster <bfoster@redhat.com> Link: https://patch.msgid.link/20251208140548.373411-1-bfoster@redhat.com Acked-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent a260bd2 commit ed61378

File tree

4 files changed

+50
-25
lines changed

4 files changed

+50
-25
lines changed

fs/iomap/buffered-io.c

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ static struct folio *__iomap_get_folio(struct iomap_iter *iter,
832832
if (!mapping_large_folio_support(iter->inode->i_mapping))
833833
len = min_t(size_t, len, PAGE_SIZE - offset_in_page(pos));
834834

835-
if (iter->fbatch) {
835+
if (iter->iomap.flags & IOMAP_F_FOLIO_BATCH) {
836836
struct folio *folio = folio_batch_next(iter->fbatch);
837837

838838
if (!folio)
@@ -929,7 +929,7 @@ static int iomap_write_begin(struct iomap_iter *iter,
929929
* process so return and let the caller iterate and refill the batch.
930930
*/
931931
if (!folio) {
932-
WARN_ON_ONCE(!iter->fbatch);
932+
WARN_ON_ONCE(!(iter->iomap.flags & IOMAP_F_FOLIO_BATCH));
933933
return 0;
934934
}
935935

@@ -1544,23 +1544,39 @@ static int iomap_zero_iter(struct iomap_iter *iter, bool *did_zero,
15441544
return status;
15451545
}
15461546

1547-
loff_t
1547+
/**
1548+
* iomap_fill_dirty_folios - fill a folio batch with dirty folios
1549+
* @iter: Iteration structure
1550+
* @start: Start offset of range. Updated based on lookup progress.
1551+
* @end: End offset of range
1552+
* @iomap_flags: Flags to set on the associated iomap to track the batch.
1553+
*
1554+
* Returns the folio count directly. Also returns the associated control flag if
1555+
* the the batch lookup is performed and the expected offset of a subsequent
1556+
* lookup via out params. The caller is responsible to set the flag on the
1557+
* associated iomap.
1558+
*/
1559+
unsigned int
15481560
iomap_fill_dirty_folios(
15491561
struct iomap_iter *iter,
1550-
loff_t offset,
1551-
loff_t length)
1562+
loff_t *start,
1563+
loff_t end,
1564+
unsigned int *iomap_flags)
15521565
{
15531566
struct address_space *mapping = iter->inode->i_mapping;
1554-
pgoff_t start = offset >> PAGE_SHIFT;
1555-
pgoff_t end = (offset + length - 1) >> PAGE_SHIFT;
1567+
pgoff_t pstart = *start >> PAGE_SHIFT;
1568+
pgoff_t pend = (end - 1) >> PAGE_SHIFT;
1569+
unsigned int count;
15561570

1557-
iter->fbatch = kmalloc(sizeof(struct folio_batch), GFP_KERNEL);
1558-
if (!iter->fbatch)
1559-
return offset + length;
1560-
folio_batch_init(iter->fbatch);
1571+
if (!iter->fbatch) {
1572+
*start = end;
1573+
return 0;
1574+
}
15611575

1562-
filemap_get_folios_dirty(mapping, &start, end, iter->fbatch);
1563-
return (start << PAGE_SHIFT);
1576+
count = filemap_get_folios_dirty(mapping, &pstart, pend, iter->fbatch);
1577+
*start = (pstart << PAGE_SHIFT);
1578+
*iomap_flags |= IOMAP_F_FOLIO_BATCH;
1579+
return count;
15641580
}
15651581
EXPORT_SYMBOL_GPL(iomap_fill_dirty_folios);
15661582

@@ -1569,17 +1585,21 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,
15691585
const struct iomap_ops *ops,
15701586
const struct iomap_write_ops *write_ops, void *private)
15711587
{
1588+
struct folio_batch fbatch;
15721589
struct iomap_iter iter = {
15731590
.inode = inode,
15741591
.pos = pos,
15751592
.len = len,
15761593
.flags = IOMAP_ZERO,
15771594
.private = private,
1595+
.fbatch = &fbatch,
15781596
};
15791597
struct address_space *mapping = inode->i_mapping;
15801598
int ret;
15811599
bool range_dirty;
15821600

1601+
folio_batch_init(&fbatch);
1602+
15831603
/*
15841604
* To avoid an unconditional flush, check pagecache state and only flush
15851605
* if dirty and the fs returns a mapping that might convert on
@@ -1590,11 +1610,11 @@ iomap_zero_range(struct inode *inode, loff_t pos, loff_t len, bool *did_zero,
15901610
while ((ret = iomap_iter(&iter, ops)) > 0) {
15911611
const struct iomap *srcmap = iomap_iter_srcmap(&iter);
15921612

1593-
if (WARN_ON_ONCE(iter.fbatch &&
1613+
if (WARN_ON_ONCE((iter.iomap.flags & IOMAP_F_FOLIO_BATCH) &&
15941614
srcmap->type != IOMAP_UNWRITTEN))
15951615
return -EIO;
15961616

1597-
if (!iter.fbatch &&
1617+
if (!(iter.iomap.flags & IOMAP_F_FOLIO_BATCH) &&
15981618
(srcmap->type == IOMAP_HOLE ||
15991619
srcmap->type == IOMAP_UNWRITTEN)) {
16001620
s64 status;

fs/iomap/iter.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88

99
static inline void iomap_iter_reset_iomap(struct iomap_iter *iter)
1010
{
11-
if (iter->fbatch) {
11+
if (iter->iomap.flags & IOMAP_F_FOLIO_BATCH) {
1212
folio_batch_release(iter->fbatch);
13-
kfree(iter->fbatch);
14-
iter->fbatch = NULL;
13+
folio_batch_reinit(iter->fbatch);
14+
iter->iomap.flags &= ~IOMAP_F_FOLIO_BATCH;
1515
}
1616

1717
iter->status = 0;

fs/xfs/xfs_iomap.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,7 +1831,6 @@ xfs_buffered_write_iomap_begin(
18311831
*/
18321832
if (flags & IOMAP_ZERO) {
18331833
xfs_fileoff_t eof_fsb = XFS_B_TO_FSB(mp, XFS_ISIZE(ip));
1834-
u64 end;
18351834

18361835
if (isnullstartblock(imap.br_startblock) &&
18371836
offset_fsb >= eof_fsb)
@@ -1851,12 +1850,14 @@ xfs_buffered_write_iomap_begin(
18511850
*/
18521851
if (imap.br_state == XFS_EXT_UNWRITTEN &&
18531852
offset_fsb < eof_fsb) {
1854-
loff_t len = min(count,
1855-
XFS_FSB_TO_B(mp, imap.br_blockcount));
1853+
loff_t foffset = offset, fend;
18561854

1857-
end = iomap_fill_dirty_folios(iter, offset, len);
1855+
fend = offset +
1856+
min(count, XFS_FSB_TO_B(mp, imap.br_blockcount));
1857+
iomap_fill_dirty_folios(iter, &foffset, fend,
1858+
&iomap_flags);
18581859
end_fsb = min_t(xfs_fileoff_t, end_fsb,
1859-
XFS_B_TO_FSB(mp, end));
1860+
XFS_B_TO_FSB(mp, foffset));
18601861
}
18611862

18621863
xfs_trim_extent(&imap, offset_fsb, end_fsb - offset_fsb);

include/linux/iomap.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,17 @@ struct vm_fault;
8888
/*
8989
* Flags set by the core iomap code during operations:
9090
*
91+
* IOMAP_F_FOLIO_BATCH indicates that the folio batch mechanism is active
92+
* for this operation, set by iomap_fill_dirty_folios().
93+
*
9194
* IOMAP_F_SIZE_CHANGED indicates to the iomap_end method that the file size
9295
* has changed as the result of this write operation.
9396
*
9497
* IOMAP_F_STALE indicates that the iomap is not valid any longer and the file
9598
* range it covers needs to be remapped by the high level before the operation
9699
* can proceed.
97100
*/
101+
#define IOMAP_F_FOLIO_BATCH (1U << 13)
98102
#define IOMAP_F_SIZE_CHANGED (1U << 14)
99103
#define IOMAP_F_STALE (1U << 15)
100104

@@ -352,8 +356,8 @@ bool iomap_dirty_folio(struct address_space *mapping, struct folio *folio);
352356
int iomap_file_unshare(struct inode *inode, loff_t pos, loff_t len,
353357
const struct iomap_ops *ops,
354358
const struct iomap_write_ops *write_ops);
355-
loff_t iomap_fill_dirty_folios(struct iomap_iter *iter, loff_t offset,
356-
loff_t length);
359+
unsigned int iomap_fill_dirty_folios(struct iomap_iter *iter, loff_t *start,
360+
loff_t end, unsigned int *iomap_flags);
357361
int iomap_zero_range(struct inode *inode, loff_t pos, loff_t len,
358362
bool *did_zero, const struct iomap_ops *ops,
359363
const struct iomap_write_ops *write_ops, void *private);

0 commit comments

Comments
 (0)