On Tue 14-09-21 10:13:04, Neil Brown wrote: > Only 4 subsystems call set_bdi_congested() or clear_bdi_congested(): > block/pktcdvd, fs/ceph fs/fuse fs/nfs > > It may make sense to use congestion_wait() or wait_iff_congested() > within these subsystems, but they have no value outside of these. > > Add documentation comments to these functions to discourage further use. This is an unfortunate state. The MM layer still relies on the API. While adding a documentation to clarify the current status can stop more usage I am wondering what is a real alternative. My experience tells me that a lack of real alternative will lead to new creative ways of doing things instead. > Signed-off-by: NeilBrown <neilb@xxxxxxx> > --- > include/linux/backing-dev.h | 7 +++++++ > mm/backing-dev.c | 9 +++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h > index ac7f231b8825..cc9513840351 100644 > --- a/include/linux/backing-dev.h > +++ b/include/linux/backing-dev.h > @@ -153,6 +153,13 @@ static inline int wb_congested(struct bdi_writeback *wb, int cong_bits) > return wb->congested & cong_bits; > } > > +/* NOTE congestion_wait() and wait_iff_congested() are > + * largely useless except as documentation. > + * congestion_wait() will (almost) always wait for the given timeout. > + * wait_iff_congested() will (almost) never wait, but will call > + * cond_resched(). > + * Were possible an alternative waiting strategy should be found. > + */ > long congestion_wait(int sync, long timeout); > long wait_iff_congested(int sync, long timeout); > > diff --git a/mm/backing-dev.c b/mm/backing-dev.c > index 4a9d4e27d0d9..53472ab38796 100644 > --- a/mm/backing-dev.c > +++ b/mm/backing-dev.c > @@ -1023,6 +1023,11 @@ EXPORT_SYMBOL(set_bdi_congested); > * Waits for up to @timeout jiffies for a backing_dev (any backing_dev) to exit > * write congestion. If no backing_devs are congested then just wait for the > * next write to be completed. > + * > + * NOTE: in the current implementation, hardly any backing_devs are ever > + * marked as congested, and write-completion is rarely reported (see calls > + * to clear_bdi_congested). So this should not be assumed to ever wake before > + * the timeout. > */ > long congestion_wait(int sync, long timeout) > { > @@ -1054,6 +1059,10 @@ EXPORT_SYMBOL(congestion_wait); > * The return value is 0 if the sleep is for the full timeout. Otherwise, > * it is the number of jiffies that were still remaining when the function > * returned. return_value == timeout implies the function did not sleep. > + * > + * NOTE: in the current implementation, hardly any backing_devs are ever > + * marked as congested, and write-completion is rarely reported (see calls > + * to clear_bdi_congested). So this should not be assumed to sleep at all. > */ > long wait_iff_congested(int sync, long timeout) > { > -- Michal Hocko SUSE Labs