Skip to content

Commit 82e60d0

Browse files
hnazakpm00
authored andcommitted
fs: fix leaked psi pressure state
When psi annotations were added to to btrfs compression reads, the psi state tracking over add_ra_bio_pages and btrfs_submit_compressed_read was faulty. A pressure state, once entered, is never left. This results in incorrectly elevated pressure, which triggers OOM kills. pflags record the *previous* memstall state when we enter a new one. The code tried to initialize pflags to 1, and then optimize the leave call when we either didn't enter a memstall, or were already inside a nested stall. However, there can be multiple PageWorkingset pages in the bio, at which point it's that path itself that enters repeatedly and overwrites pflags. This causes us to miss the exit. Enter the stall only once if needed, then unwind correctly. erofs has the same problem, fix that up too. And move the memstall exit past submit_bio() to restore submit accounting originally added by b8e24a9 ("block: annotate refault stalls from IO submission"). Link: https://lkml.kernel.org/r/Y2UHRqthNUwuIQGS@cmpxchg.org Fixes: 4088a47 ("btrfs: add manual PSI accounting for compressed reads") Fixes: 99486c5 ("erofs: add manual PSI accounting for the compressed address space") Fixes: 118f366 ("block: remove PSI accounting from the bio layer") Link: https://lore.kernel.org/r/d20a0a85-e415-cf78-27f9-77dd7a94bc8d@leemhuis.info/ Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Reported-by: Thorsten Leemhuis <linux@leemhuis.info> Tested-by: Thorsten Leemhuis <linux@leemhuis.info> Cc: Chao Yu <chao@kernel.org> Cc: Chris Mason <clm@fb.com> Cc: Christoph Hellwig <hch@lst.de> Cc: David Sterba <dsterba@suse.com> Cc: Gao Xiang <xiang@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: Josef Bacik <josef@toxicpanda.com> Cc: Suren Baghdasaryan <surenb@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent 8cccf05 commit 82e60d0

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed

fs/btrfs/compression.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ static u64 bio_end_offset(struct bio *bio)
512512
static noinline int add_ra_bio_pages(struct inode *inode,
513513
u64 compressed_end,
514514
struct compressed_bio *cb,
515-
unsigned long *pflags)
515+
int *memstall, unsigned long *pflags)
516516
{
517517
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
518518
unsigned long end_index;
@@ -581,8 +581,10 @@ static noinline int add_ra_bio_pages(struct inode *inode,
581581
continue;
582582
}
583583

584-
if (PageWorkingset(page))
584+
if (!*memstall && PageWorkingset(page)) {
585585
psi_memstall_enter(pflags);
586+
*memstall = 1;
587+
}
586588

587589
ret = set_page_extent_mapped(page);
588590
if (ret < 0) {
@@ -670,8 +672,8 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
670672
u64 em_len;
671673
u64 em_start;
672674
struct extent_map *em;
673-
/* Initialize to 1 to make skip psi_memstall_leave unless needed */
674-
unsigned long pflags = 1;
675+
unsigned long pflags;
676+
int memstall = 0;
675677
blk_status_t ret;
676678
int ret2;
677679
int i;
@@ -727,7 +729,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
727729
goto fail;
728730
}
729731

730-
add_ra_bio_pages(inode, em_start + em_len, cb, &pflags);
732+
add_ra_bio_pages(inode, em_start + em_len, cb, &memstall, &pflags);
731733

732734
/* include any pages we added in add_ra-bio_pages */
733735
cb->len = bio->bi_iter.bi_size;
@@ -807,7 +809,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
807809
}
808810
}
809811

810-
if (!pflags)
812+
if (memstall)
811813
psi_memstall_leave(&pflags);
812814

813815
if (refcount_dec_and_test(&cb->pending_ios))

fs/erofs/zdata.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,8 +1412,8 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f,
14121412
struct block_device *last_bdev;
14131413
unsigned int nr_bios = 0;
14141414
struct bio *bio = NULL;
1415-
/* initialize to 1 to make skip psi_memstall_leave unless needed */
1416-
unsigned long pflags = 1;
1415+
unsigned long pflags;
1416+
int memstall = 0;
14171417

14181418
bi_private = jobqueueset_init(sb, q, fgq, force_fg);
14191419
qtail[JQ_BYPASS] = &q[JQ_BYPASS]->head;
@@ -1463,14 +1463,18 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f,
14631463
if (bio && (cur != last_index + 1 ||
14641464
last_bdev != mdev.m_bdev)) {
14651465
submit_bio_retry:
1466-
if (!pflags)
1467-
psi_memstall_leave(&pflags);
14681466
submit_bio(bio);
1467+
if (memstall) {
1468+
psi_memstall_leave(&pflags);
1469+
memstall = 0;
1470+
}
14691471
bio = NULL;
14701472
}
14711473

1472-
if (unlikely(PageWorkingset(page)))
1474+
if (unlikely(PageWorkingset(page)) && !memstall) {
14731475
psi_memstall_enter(&pflags);
1476+
memstall = 1;
1477+
}
14741478

14751479
if (!bio) {
14761480
bio = bio_alloc(mdev.m_bdev, BIO_MAX_VECS,
@@ -1500,9 +1504,9 @@ static void z_erofs_submit_queue(struct z_erofs_decompress_frontend *f,
15001504
} while (owned_head != Z_EROFS_PCLUSTER_TAIL);
15011505

15021506
if (bio) {
1503-
if (!pflags)
1504-
psi_memstall_leave(&pflags);
15051507
submit_bio(bio);
1508+
if (memstall)
1509+
psi_memstall_leave(&pflags);
15061510
}
15071511

15081512
/*

0 commit comments

Comments
 (0)