* Re: [RFC] writeback: add elastic bdi in cgwb bdp
2019-10-12 13:27 [RFC] writeback: add elastic bdi in cgwb bdp Hillf Danton
@ 2019-10-15 10:22 ` Jan Kara
2019-10-15 14:03 ` Hillf Danton
1 sibling, 0 replies; 5+ messages in thread
From: Jan Kara @ 2019-10-15 10:22 UTC (permalink / raw)
To: Hillf Danton
Cc: mm, fsdev, Andrew Morton, linux, Roman Gushchin, Tejun Heo,
Jan Kara, Johannes Weiner, Shakeel Butt, Minchan Kim, Mel Gorman
Hello,
On Sat 12-10-19 21:27:40, Hillf Danton wrote:
> The behaviors of the elastic bdi (ebdi) observed in the current cgwb
> bandwidth measurement include
>
> 1, like spinning disks on market ebdi can do ~128MB/s IOs in consective
> minutes in few scenarios, or higher like SSD, or lower like USB key.
>
> 2, with ebdi a bdi_writeback, wb-A, is able to do 80MB/s writeouts in the
> current time window of 200ms, while it was 16M/s in the previous one.
>
> 3, it will be either 100MB/s in the next time window if wb-B joins wb-A
> writing pages out or 18MB/s if wb-C also decides to chime in.
>
> With the help of bandwidth gauged above, what is left in balancing dirty
> pages, bdp, is try to make wb-A's laundry speed catch up dirty speed in
> every 200ms interval without knowing what wb-B is doing.
>
> No heuristic is added in this work because ebdi does bdp without it.
Thanks for the patch but honestly, I have hard time understanding what is
the purpose of this patch from the changelog. Some kind of writeback
throttling? And why is this needed? Also some highlevel description of what
your solution is would be good...
Honza
> Cc: Roman Gushchin <guro@fb.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Shakeel Butt <shakeelb@google.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Mel Gorman <mgorman@suse.de>
> Signed-off-by: Hillf Danton <hdanton@sina.com>
> ---
>
> --- a/include/linux/backing-dev-defs.h
> +++ b/include/linux/backing-dev-defs.h
> @@ -157,6 +157,9 @@ struct bdi_writeback {
> struct list_head memcg_node; /* anchored at memcg->cgwb_list */
> struct list_head blkcg_node; /* anchored at blkcg->cgwb_list */
>
> +#ifdef CONFIG_CGWB_BDP_WITH_EBDI
> + struct wait_queue_head bdp_waitq;
> +#endif
> union {
> struct work_struct release_work;
> struct rcu_head rcu;
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -324,6 +324,10 @@ static int wb_init(struct bdi_writeback
> goto out_destroy_stat;
> }
>
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> + init_waitqueue_head(&wb->bdp_waitq);
> +
> return 0;
>
> out_destroy_stat:
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1551,6 +1551,45 @@ static inline void wb_dirty_limits(struc
> }
> }
>
> +#if defined(CONFIG_CGROUP_WRITEBACK) && defined(CONFIG_CGWB_BDP_WITH_EBDI)
> +static bool cgwb_bdp_should_throttle(struct bdi_writeback *wb)
> +{
> + struct dirty_throttle_control gdtc = { GDTC_INIT_NO_WB };
> +
> + if (fatal_signal_pending(current))
> + return false;
> +
> + gdtc.avail = global_dirtyable_memory();
> +
> + domain_dirty_limits(&gdtc);
> +
> + gdtc.dirty = global_node_page_state(NR_FILE_DIRTY) +
> + global_node_page_state(NR_UNSTABLE_NFS) +
> + global_node_page_state(NR_WRITEBACK);
> +
> + if (gdtc.dirty < gdtc.bg_thresh)
> + return false;
> +
> + if (!writeback_in_progress(wb))
> + wb_start_background_writeback(wb);
> +
> + /*
> + * throttle if laundry speed remarkably falls behind dirty speed
> + * in the current time window of 200ms
> + */
> + return gdtc.dirty > gdtc.thresh &&
> + wb_stat(wb, WB_DIRTIED) >
> + wb_stat(wb, WB_WRITTEN) +
> + wb_stat_error();
> +}
> +
> +static inline void cgwb_bdp(struct bdi_writeback *wb)
> +{
> + wait_event_interruptible_timeout(wb->bdp_waitq,
> + !cgwb_bdp_should_throttle(wb), HZ);
> +}
> +#endif
> +
> /*
> * balance_dirty_pages() must be called by processes which are generating dirty
> * data. It looks at the number of dirty pages in the machine and will force
> @@ -1910,7 +1949,11 @@ void balance_dirty_pages_ratelimited(str
> preempt_enable();
>
> if (unlikely(current->nr_dirtied >= ratelimit))
> - balance_dirty_pages(wb, current->nr_dirtied);
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> + cgwb_bdp(wb);
> + else
> + balance_dirty_pages(wb, current->nr_dirtied);
>
> wb_put(wb);
> }
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -632,6 +632,11 @@ void wbc_detach_inode(struct writeback_c
> if (!wb)
> return;
>
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> + if (waitqueue_active(&wb->bdp_waitq))
> + wake_up_all(&wb->bdp_waitq);
> +
> history = inode->i_wb_frn_history;
> avg_time = inode->i_wb_frn_avg_time;
>
> @@ -811,6 +816,9 @@ static long wb_split_bdi_pages(struct bd
> if (nr_pages == LONG_MAX)
> return LONG_MAX;
>
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> + return nr_pages;
> /*
> * This may be called on clean wb's and proportional distribution
> * may not make sense, just use the original @nr_pages in those
> @@ -1599,6 +1607,10 @@ static long writeback_chunk_size(struct
> if (work->sync_mode == WB_SYNC_ALL || work->tagged_writepages)
> pages = LONG_MAX;
> else {
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> + return work->nr_pages;
> +
> pages = min(wb->avg_write_bandwidth / 2,
> global_wb_domain.dirty_limit / DIRTY_SCOPE);
> pages = min(pages, work->nr_pages);
> --
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC] writeback: add elastic bdi in cgwb bdp
2019-10-12 13:27 [RFC] writeback: add elastic bdi in cgwb bdp Hillf Danton
2019-10-15 10:22 ` Jan Kara
@ 2019-10-15 14:03 ` Hillf Danton
2019-10-15 14:37 ` Tejun Heo
2019-10-16 2:26 ` Hillf Danton
1 sibling, 2 replies; 5+ messages in thread
From: Hillf Danton @ 2019-10-15 14:03 UTC (permalink / raw)
To: Jan Kara
Cc: mm, fsdev, Andrew Morton, linux, Roman Gushchin, Tejun Heo,
Johannes Weiner, Shakeel Butt, Fengguang Wu, Hillf Danton,
Minchan Kim, Mel Gorman
Hey Jan
On Tue, 15 Oct 2019 12:22:10 +0200 Jan Kara wrote:
> Hello,
>
> On Sat 12-10-19 21:27:40, Hillf Danton wrote:
> >
> > The behaviors of the elastic bdi (ebdi) observed in the current cgwb
> > bandwidth measurement include
> >
> > 1, like spinning disks on market ebdi can do ~128MB/s IOs in consective
> > minutes in few scenarios, or higher like SSD, or lower like USB key.
> >
> > 2, with ebdi a bdi_writeback, wb-A, is able to do 80MB/s writeouts in the
> > current time window of 200ms, while it was 16M/s in the previous one.
> >
> > 3, it will be either 100MB/s in the next time window if wb-B joins wb-A
> > writing pages out or 18MB/s if wb-C also decides to chime in.
> >
> > With the help of bandwidth gauged above, what is left in balancing dirty
> > pages, bdp, is try to make wb-A's laundry speed catch up dirty speed in
> > every 200ms interval without knowing what wb-B is doing.
> >
> > No heuristic is added in this work because ebdi does bdp without it.
>
> Thanks for the patch but honestly, I have hard time understanding what is
> the purpose of this patch from the changelog.
Fault on my side. I will try to make it as clear as I could.
Under the cover of "behaviors of elastic bdi" I list the difficulties in
the current writeback bandwidth measurings, particularly in the case with
CONFIG_CGROUP_WRITEBACK enabled, with the phrase ebdi used to abstract
the attribute of hardwares, like spinning disk, SSD and USB storage,
that their physical bandwidth is a constant.
The difference between that constant and the bandwidth currently
measured comes from, I think, the IO pattern dispatched to hardware in
the time interval of 200ms. How much sense does it make to guide wb-A's
IO in the next 200ms without idea about what other wbs are doing? What
should be modeled and built on top of the measured bw value?
Hard to say.
What will bdp OTOH look like on top of ebdi without the hard work of
measuring bw?
A name came up before I am tapping this message, though not available
when I sent the RFC, and it essentially is that ebdi paves a brick for
applying the walk-dog method to bdp: let wb-A's laundry speed walk its
dirty speed the same way as pet owners in Paris, Prague and other cities
go walking their dogs every day with a leash worth two dimes on average.
Is a $200 electronic walkmeter needed to have a good time of walking dog
in London?
Nope, I think because it makes ant-eyelash-size sense to gauge the walker's
speed first with that gadget prone to glitch and then teach the dog to
walk that speed, and to do more based on it.
The only reason I have to do walk-dog in bdp is that laundry speed
remarkably falls behind dirty speed in every case of bdp with no exception.
And a leash is supposed to do the job in a manner that it should naturally
be, even though laundry speed changes in every 200ms interval and is
usually hard to predict before hand under real workloads, with two things
below met in every 200ms:
1, dirty pages in the system clamped near the threshold that is configurable
in userspace,
2, dirty speed of every wb glued as close to the laundry speed as possible,
in long run.
Should walk-dog be in place, then we can do cleanups of bw measurement and
things dependent of it a step after another.
Thanks
Hillf
> Some kind of writeback throttling?
> And why is this needed?
> Also some highlevel description of what
> your solution is would be good...
>
> Honza
>
> > Cc: Roman Gushchin <guro@fb.com>
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Shakeel Butt <shakeelb@google.com>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Signed-off-by: Hillf Danton <hdanton@sina.com>
> > ---
> >
> > --- a/include/linux/backing-dev-defs.h
> > +++ b/include/linux/backing-dev-defs.h
> > @@ -157,6 +157,9 @@ struct bdi_writeback {
> > struct list_head memcg_node; /* anchored at memcg->cgwb_list */
> > struct list_head blkcg_node; /* anchored at blkcg->cgwb_list */
> >
> > +#ifdef CONFIG_CGWB_BDP_WITH_EBDI
> > + struct wait_queue_head bdp_waitq;
> > +#endif
> > union {
> > struct work_struct release_work;
> > struct rcu_head rcu;
> > --- a/mm/backing-dev.c
> > +++ b/mm/backing-dev.c
> > @@ -324,6 +324,10 @@ static int wb_init(struct bdi_writeback
> > goto out_destroy_stat;
> > }
> >
> > + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> > + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> > + init_waitqueue_head(&wb->bdp_waitq);
> > +
> > return 0;
> >
> > out_destroy_stat:
> > --- a/mm/page-writeback.c
> > +++ b/mm/page-writeback.c
> > @@ -1551,6 +1551,45 @@ static inline void wb_dirty_limits(struc
> > }
> > }
> >
> > +#if defined(CONFIG_CGROUP_WRITEBACK) && defined(CONFIG_CGWB_BDP_WITH_EBDI)
> > +static bool cgwb_bdp_should_throttle(struct bdi_writeback *wb)
> > +{
> > + struct dirty_throttle_control gdtc = { GDTC_INIT_NO_WB };
> > +
> > + if (fatal_signal_pending(current))
> > + return false;
> > +
> > + gdtc.avail = global_dirtyable_memory();
> > +
> > + domain_dirty_limits(&gdtc);
> > +
> > + gdtc.dirty = global_node_page_state(NR_FILE_DIRTY) +
> > + global_node_page_state(NR_UNSTABLE_NFS) +
> > + global_node_page_state(NR_WRITEBACK);
> > +
> > + if (gdtc.dirty < gdtc.bg_thresh)
> > + return false;
> > +
> > + if (!writeback_in_progress(wb))
> > + wb_start_background_writeback(wb);
> > +
> > + /*
> > + * throttle if laundry speed remarkably falls behind dirty speed
> > + * in the current time window of 200ms
> > + */
> > + return gdtc.dirty > gdtc.thresh &&
> > + wb_stat(wb, WB_DIRTIED) >
> > + wb_stat(wb, WB_WRITTEN) +
> > + wb_stat_error();
> > +}
> > +
> > +static inline void cgwb_bdp(struct bdi_writeback *wb)
> > +{
> > + wait_event_interruptible_timeout(wb->bdp_waitq,
> > + !cgwb_bdp_should_throttle(wb), HZ);
> > +}
> > +#endif
> > +
> > /*
> > * balance_dirty_pages() must be called by processes which are generating dirty
> > * data. It looks at the number of dirty pages in the machine and will force
> > @@ -1910,7 +1949,11 @@ void balance_dirty_pages_ratelimited(str
> > preempt_enable();
> >
> > if (unlikely(current->nr_dirtied >= ratelimit))
> > - balance_dirty_pages(wb, current->nr_dirtied);
> > + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> > + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> > + cgwb_bdp(wb);
> > + else
> > + balance_dirty_pages(wb, current->nr_dirtied);
> >
> > wb_put(wb);
> > }
> > --- a/fs/fs-writeback.c
> > +++ b/fs/fs-writeback.c
> > @@ -632,6 +632,11 @@ void wbc_detach_inode(struct writeback_c
> > if (!wb)
> > return;
> >
> > + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> > + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> > + if (waitqueue_active(&wb->bdp_waitq))
> > + wake_up_all(&wb->bdp_waitq);
> > +
> > history = inode->i_wb_frn_history;
> > avg_time = inode->i_wb_frn_avg_time;
> >
> > @@ -811,6 +816,9 @@ static long wb_split_bdi_pages(struct bd
> > if (nr_pages == LONG_MAX)
> > return LONG_MAX;
> >
> > + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> > + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> > + return nr_pages;
> > /*
> > * This may be called on clean wb's and proportional distribution
> > * may not make sense, just use the original @nr_pages in those
> > @@ -1599,6 +1607,10 @@ static long writeback_chunk_size(struct
> > if (work->sync_mode == WB_SYNC_ALL || work->tagged_writepages)
> > pages = LONG_MAX;
> > else {
> > + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) &&
> > + IS_ENABLED(CONFIG_CGWB_BDP_WITH_EBDI))
> > + return work->nr_pages;
> > +
> > pages = min(wb->avg_write_bandwidth / 2,
> > global_wb_domain.dirty_limit / DIRTY_SCOPE);
> > pages = min(pages, work->nr_pages);
> > --
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread