| Message ID | 20220910065058.3303831-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] mm: add PSI accounting around ->read_folio and ->readahead calls | expand |
On Sat, Sep 10, 2022 at 08:50:56AM +0200, Christoph Hellwig wrote: > btrfs compressed reads try to always read the entire compressed chunk, > even if only a subset is requested. Currently this is covered by the > magic PSI accounting underneath submit_bio, but that is about to go > away. Instead add manual psi_memstall_{enter,leave} annotations. > > Note that for readahead this really should be using readahead_expand, > but the additionals reads are also done for plain ->read_folio where > readahead_expand can't work, so this overall logic is left as-is for > now. > > Signed-off-by: Christoph Hellwig <hch@lst.de> With some small fixups, Acked-by: David Sterba <dsterba@suse.com> > fs/btrfs/compression.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c > index e84d22c5c6a83..f7889a00e0055 100644 > --- a/fs/btrfs/compression.c > +++ b/fs/btrfs/compression.c > @@ -15,6 +15,7 @@ > #include <linux/string.h> > #include <linux/backing-dev.h> > #include <linux/writeback.h> > +#include <linux/psi.h> > #include <linux/slab.h> > #include <linux/sched/mm.h> > #include <linux/log2.h> > @@ -519,7 +520,8 @@ static u64 bio_end_offset(struct bio *bio) > */ > static noinline int add_ra_bio_pages(struct inode *inode, > u64 compressed_end, > - struct compressed_bio *cb) > + struct compressed_bio *cb, > + unsigned long *pflags) > { > struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); > unsigned long end_index; > @@ -588,6 +590,9 @@ static noinline int add_ra_bio_pages(struct inode *inode, > continue; > } > > + if (unlikely(PageWorkingset(page))) Please drop the 'unlikely', in this case it does not seem to make much sense. > + psi_memstall_enter(pflags); > + > ret = set_page_extent_mapped(page); > if (ret < 0) { > unlock_page(page); > @@ -674,6 +679,8 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > u64 em_len; > u64 em_start; > struct extent_map *em; > + /* initialize to 1 to make skip psi_memstall_leave unless needed */ First letter in comment should be upper case unless it's an identifier. > + unsigned long pflags = 1; > blk_status_t ret; > int ret2; > int i; > @@ -729,7 +736,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > goto fail; > } > > - add_ra_bio_pages(inode, em_start + em_len, cb); > + add_ra_bio_pages(inode, em_start + em_len, cb, &pflags); > > /* include any pages we added in add_ra-bio_pages */ > cb->len = bio->bi_iter.bi_size; > @@ -810,6 +817,9 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, > } > } > > + if (!pflags) > + psi_memstall_leave(&pflags); > + > if (refcount_dec_and_test(&cb->pending_ios)) > finish_compressed_bio_read(cb); > return; > -- > 2.30.2 >
On Sat, Sep 10, 2022 at 08:50:56AM +0200, Christoph Hellwig wrote: > btrfs compressed reads try to always read the entire compressed chunk, > even if only a subset is requested. Currently this is covered by the > magic PSI accounting underneath submit_bio, but that is about to go > away. Instead add manual psi_memstall_{enter,leave} annotations. > > Note that for readahead this really should be using readahead_expand, > but the additionals reads are also done for plain ->read_folio where > readahead_expand can't work, so this overall logic is left as-is for > now. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c index e84d22c5c6a83..f7889a00e0055 100644 --- a/fs/btrfs/compression.c +++ b/fs/btrfs/compression.c @@ -15,6 +15,7 @@ #include <linux/string.h> #include <linux/backing-dev.h> #include <linux/writeback.h> +#include <linux/psi.h> #include <linux/slab.h> #include <linux/sched/mm.h> #include <linux/log2.h> @@ -519,7 +520,8 @@ static u64 bio_end_offset(struct bio *bio) */ static noinline int add_ra_bio_pages(struct inode *inode, u64 compressed_end, - struct compressed_bio *cb) + struct compressed_bio *cb, + unsigned long *pflags) { struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); unsigned long end_index; @@ -588,6 +590,9 @@ static noinline int add_ra_bio_pages(struct inode *inode, continue; } + if (unlikely(PageWorkingset(page))) + psi_memstall_enter(pflags); + ret = set_page_extent_mapped(page); if (ret < 0) { unlock_page(page); @@ -674,6 +679,8 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, u64 em_len; u64 em_start; struct extent_map *em; + /* initialize to 1 to make skip psi_memstall_leave unless needed */ + unsigned long pflags = 1; blk_status_t ret; int ret2; int i; @@ -729,7 +736,7 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, goto fail; } - add_ra_bio_pages(inode, em_start + em_len, cb); + add_ra_bio_pages(inode, em_start + em_len, cb, &pflags); /* include any pages we added in add_ra-bio_pages */ cb->len = bio->bi_iter.bi_size; @@ -810,6 +817,9 @@ void btrfs_submit_compressed_read(struct inode *inode, struct bio *bio, } } + if (!pflags) + psi_memstall_leave(&pflags); + if (refcount_dec_and_test(&cb->pending_ios)) finish_compressed_bio_read(cb); return;
btrfs compressed reads try to always read the entire compressed chunk, even if only a subset is requested. Currently this is covered by the magic PSI accounting underneath submit_bio, but that is about to go away. Instead add manual psi_memstall_{enter,leave} annotations. Note that for readahead this really should be using readahead_expand, but the additionals reads are also done for plain ->read_folio where readahead_expand can't work, so this overall logic is left as-is for now. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/btrfs/compression.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)