On Mon 03-03-25 18:06:17, Tang Yizhou wrote:
> From: Tang Yizhou <yizhou.tang@xxxxxxxxxx>
>
> In the commit dcc25ae76eb7 ("writeback: move global_dirty_limit into
> wb_domain") of the cgroup writeback backpressure propagation patchset,
> Tejun made some adaptations to trace_balance_dirty_pages() for cgroup
> writeback. However, this adaptation was incomplete and Tejun missed
> further adaptation in the subsequent patches.
>
> In the cgroup writeback scenario, if sdtc in balance_dirty_pages() is
> assigned to mdtc, then upon entering trace_balance_dirty_pages(),
> __entry->limit should be assigned based on the dirty_limit of the
> corresponding memcg's wb_domain, rather than global_wb_domain.
>
> To address this issue and simplify the implementation, introduce a 'limit'
> field in struct dirty_throttle_control to store the hard_limit value
> computed in wb_position_ratio() by calling hard_dirty_limit(). This field
> will then be used in trace_balance_dirty_pages() to assign the value to
> __entry->limit.
>
> Fixes: dcc25ae76eb7 ("writeback: move global_dirty_limit into wb_domain")
> Signed-off-by: Tang Yizhou <yizhou.tang@xxxxxxxxxx>
In principle this looks fine but one nit below:
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index 32095928365c..58bda3347914 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -326,6 +326,7 @@ struct dirty_throttle_control {
> unsigned long dirty; /* file_dirty + write + nfs */
> unsigned long thresh; /* dirty threshold */
> unsigned long bg_thresh; /* dirty background threshold */
> + unsigned long limit; /* hard dirty limit */
^^^ I'd call this dirty_limit to not invent
a new name for the same thing. I've noticed the tracepoint has 'limit' as
well but that is the outlier that should be modified if anything. Also I'd
modify the comment to /* smoothed dirty limit */ to better explain what
this is about.
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
