On Fri 02-11-18 16:31:06, Arnd Bergmann wrote:
> In a rare randconfig build, I got a warning about possibly uninitialized
> variables:
>
> mm/page-writeback.c: In function 'balance_dirty_pages':
> mm/page-writeback.c:1623:16: error: 'writeback' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> mdtc->dirty += writeback;
> ^~
> mm/page-writeback.c:1624:4: error: 'filepages' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> mdtc_calc_avail(mdtc, filepages, headroom);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mm/page-writeback.c:1624:4: error: 'headroom' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The compiler evidently fails to notice that the usage is in dead code
> after 'mdtc' is set to NULL when CONFIG_CGROUP_WRITEBACK is disabled.
> Adding an IS_ENABLED() check makes this clear to the compiler.
>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
I'm surprised the compiler was not able to infer this since:
struct dirty_throttle_control * const mdtc = mdtc_valid(&mdtc_stor) ?
&mdtc_stor : NULL;
and if CONFIG_CGROUP_WRITEBACK is disabled, mdtc_valid() is defined to
'false'. But possibly the function is just too big and the problematic
condition is in the loop so maybe it all confuses the compiler too much.
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 3f690bae6b78..f02535b7731a 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -1611,7 +1611,7 @@ static void balance_dirty_pages(struct bdi_writeback *wb,
> bg_thresh = gdtc->bg_thresh;
> }
>
> - if (mdtc) {
> + if (IS_ENABLED(CONFIG_CGROUP_WRITEBACK) && mdtc) {
> unsigned long filepages, headroom, writeback;
Honestly, I don't like the IS_ENABLED(CONFIG_CGROUP_WRITEBACK) check here.
It just looks too arbitrary. Could we perhaps change the code like
struct dirty_throttle_control * const mdtc = &mdtc_stor;
And then replace checks for !mtdc in the function to !mdtc_valid(mdtc)?
That is the same thing as currently and it should make it obvious to the
compiler as well as human what is going on... Tejun?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
