| Message ID | 163936886727.23860.5245364396572576756.stgit@noble.brown (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove some 'congested' tests | expand |
On Mon, Dec 13, 2021 at 03:14:27PM +1100, NeilBrown wrote: > These functions are no longer useful as the only bdis that report > congestion are in ceph, fuse, and nfs. None of those bdis can be the > target of the calls in drbd, ext2, nilfs2, or xfs. > > Removing the test on bdi_write_contested() in current_may_throttle() > could cause a small change in behaviour, but only when PF_LOCAL_THROTTLE > is set. > > So replace the calls by 'false' and simplify the code - and remove the > functions. > > Signed-off-by: NeilBrown <neilb@suse.de> .... > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > index 631c5a61d89b..22f73b3e888e 100644 > --- a/fs/xfs/xfs_buf.c > +++ b/fs/xfs/xfs_buf.c > @@ -843,9 +843,6 @@ xfs_buf_readahead_map( > { > struct xfs_buf *bp; > > - if (bdi_read_congested(target->bt_bdev->bd_disk->bdi)) > - return; Ok, but this isn't a "throttle writeback" test here - it's trying to avoid having speculative readahead blocking on a full request queue instead of just skipping the readahead IO. i.e. prevent readahead thrashing and/or adding unnecessary read load when we already have a full read queue... So what is the replacement for that? We want to skip the entire buffer lookup/setup/read overhead if we're likely to block on IO submission - is there anything we can use to do this these days? Cheers, Dave.
On Mon, 13 Dec 2021, Dave Chinner wrote: > On Mon, Dec 13, 2021 at 03:14:27PM +1100, NeilBrown wrote: > > These functions are no longer useful as the only bdis that report > > congestion are in ceph, fuse, and nfs. None of those bdis can be the > > target of the calls in drbd, ext2, nilfs2, or xfs. > > > > Removing the test on bdi_write_contested() in current_may_throttle() > > could cause a small change in behaviour, but only when PF_LOCAL_THROTTLE > > is set. > > > > So replace the calls by 'false' and simplify the code - and remove the > > functions. > > > > Signed-off-by: NeilBrown <neilb@suse.de> > .... > > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c > > index 631c5a61d89b..22f73b3e888e 100644 > > --- a/fs/xfs/xfs_buf.c > > +++ b/fs/xfs/xfs_buf.c > > @@ -843,9 +843,6 @@ xfs_buf_readahead_map( > > { > > struct xfs_buf *bp; > > > > - if (bdi_read_congested(target->bt_bdev->bd_disk->bdi)) > > - return; > > Ok, but this isn't a "throttle writeback" test here - it's trying to > avoid having speculative readahead blocking on a full request queue > instead of just skipping the readahead IO. i.e. prevent readahead > thrashing and/or adding unnecessary read load when we already have a > full read queue... > > So what is the replacement for that? We want to skip the entire > buffer lookup/setup/read overhead if we're likely to block on IO > submission - is there anything we can use to do this these days? I don't think there is a concept of a "full read queue" any more. There are things that can block an IO submission though. There is allocation of the bio from a mempool, and there is rq_qos_throttle, and there are probably other places where submission can block. I don't think you can tell in advance if a submission is likely to block. I think the idea is that the top level of the submission stack should rate-limit based on the estimated throughput of the stack. I think write-back does this. I don't know about read-ahead. NeilBrown
diff --git a/drivers/block/drbd/drbd_int.h b/drivers/block/drbd/drbd_int.h index f27d5b0f9a0b..f804b1bfb3e6 100644 --- a/drivers/block/drbd/drbd_int.h +++ b/drivers/block/drbd/drbd_int.h @@ -638,9 +638,6 @@ enum { STATE_SENT, /* Do not change state/UUIDs while this is set */ CALLBACK_PENDING, /* Whether we have a call_usermodehelper(, UMH_WAIT_PROC) * pending, from drbd worker context. - * If set, bdi_write_congested() returns true, - * so shrink_page_list() would not recurse into, - * and potentially deadlock on, this drbd worker. */ DISCONNECT_SENT, diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c index 3235532ae077..2e5fb7e442e3 100644 --- a/drivers/block/drbd/drbd_req.c +++ b/drivers/block/drbd/drbd_req.c @@ -909,8 +909,7 @@ static bool remote_due_to_read_balancing(struct drbd_device *device, sector_t se switch (rbm) { case RB_CONGESTED_REMOTE: - return bdi_read_congested( - device->ldev->backing_bdev->bd_disk->bdi); + return 0; case RB_LEAST_PENDING: return atomic_read(&device->local_cnt) > atomic_read(&device->ap_pending_cnt) + atomic_read(&device->rs_pending_cnt); diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c index df14e750e9fe..d632764da240 100644 --- a/fs/ext2/ialloc.c +++ b/fs/ext2/ialloc.c @@ -173,8 +173,6 @@ static void ext2_preread_inode(struct inode *inode) struct backing_dev_info *bdi; bdi = inode_to_bdi(inode); - if (bdi_rw_congested(bdi)) - return; block_group = (inode->i_ino - 1) / EXT2_INODES_PER_GROUP(inode->i_sb); gdp = ext2_get_group_desc(inode->i_sb, block_group, NULL); diff --git a/fs/nilfs2/segbuf.c b/fs/nilfs2/segbuf.c index 43287b0d3e9b..d1ebc9da7130 100644 --- a/fs/nilfs2/segbuf.c +++ b/fs/nilfs2/segbuf.c @@ -343,17 +343,6 @@ static int nilfs_segbuf_submit_bio(struct nilfs_segment_buffer *segbuf, struct bio *bio = wi->bio; int err; - if (segbuf->sb_nbio > 0 && - bdi_write_congested(segbuf->sb_super->s_bdi)) { - wait_for_completion(&segbuf->sb_bio_event); - segbuf->sb_nbio--; - if (unlikely(atomic_read(&segbuf->sb_err))) { - bio_put(bio); - err = -EIO; - goto failed; - } - } - bio->bi_end_io = nilfs_end_bio_write; bio->bi_private = segbuf; bio_set_op_attrs(bio, mode, mode_flags); diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c index 631c5a61d89b..22f73b3e888e 100644 --- a/fs/xfs/xfs_buf.c +++ b/fs/xfs/xfs_buf.c @@ -843,9 +843,6 @@ xfs_buf_readahead_map( { struct xfs_buf *bp; - if (bdi_read_congested(target->bt_bdev->bd_disk->bdi)) - return; - xfs_buf_read_map(target, map, nmaps, XBF_TRYLOCK | XBF_ASYNC | XBF_READ_AHEAD, &bp, ops, __this_address); diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h index 860b675c2929..2d764566280c 100644 --- a/include/linux/backing-dev.h +++ b/include/linux/backing-dev.h @@ -135,11 +135,6 @@ static inline bool writeback_in_progress(struct bdi_writeback *wb) struct backing_dev_info *inode_to_bdi(struct inode *inode); -static inline int wb_congested(struct bdi_writeback *wb, int cong_bits) -{ - return wb->congested & cong_bits; -} - long congestion_wait(int sync, long timeout); static inline bool mapping_can_writeback(struct address_space *mapping) @@ -391,27 +386,6 @@ static inline void wb_blkcg_offline(struct blkcg *blkcg) #endif /* CONFIG_CGROUP_WRITEBACK */ -static inline int bdi_congested(struct backing_dev_info *bdi, int cong_bits) -{ - return wb_congested(&bdi->wb, cong_bits); -} - -static inline int bdi_read_congested(struct backing_dev_info *bdi) -{ - return bdi_congested(bdi, 1 << WB_sync_congested); -} - -static inline int bdi_write_congested(struct backing_dev_info *bdi) -{ - return bdi_congested(bdi, 1 << WB_async_congested); -} - -static inline int bdi_rw_congested(struct backing_dev_info *bdi) -{ - return bdi_congested(bdi, (1 << WB_sync_congested) | - (1 << WB_async_congested)); -} - const char *bdi_dev_name(struct backing_dev_info *bdi); #endif /* _LINUX_BACKING_DEV_H */ diff --git a/mm/vmscan.c b/mm/vmscan.c index 540aa0ea67ff..f46a7a17dc49 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -2321,9 +2321,7 @@ static unsigned int move_pages_to_lru(struct lruvec *lruvec, */ static int current_may_throttle(void) { - return !(current->flags & PF_LOCAL_THROTTLE) || - current->backing_dev_info == NULL || - bdi_write_congested(current->backing_dev_info); + return !(current->flags & PF_LOCAL_THROTTLE); } /*
These functions are no longer useful as the only bdis that report congestion are in ceph, fuse, and nfs. None of those bdis can be the target of the calls in drbd, ext2, nilfs2, or xfs. Removing the test on bdi_write_contested() in current_may_throttle() could cause a small change in behaviour, but only when PF_LOCAL_THROTTLE is set. So replace the calls by 'false' and simplify the code - and remove the functions. Signed-off-by: NeilBrown <neilb@suse.de> --- drivers/block/drbd/drbd_int.h | 3 --- drivers/block/drbd/drbd_req.c | 3 +-- fs/ext2/ialloc.c | 2 -- fs/nilfs2/segbuf.c | 11 ----------- fs/xfs/xfs_buf.c | 3 --- include/linux/backing-dev.h | 26 -------------------------- mm/vmscan.c | 4 +--- 7 files changed, 2 insertions(+), 50 deletions(-)