Re: [PATCH] mm: print a warning once the vm dirtiness settings is illogical

[Michal Hocko <mhocko@suse.com>]

On Mon 27-11-17 16:29:13, Yafang Shao wrote:
> 2017-11-27 16:21 GMT+08:00 Michal Hocko <mhocko@suse.com>:
> > On Mon 27-11-17 16:06:50, Yafang Shao wrote:
> >> +cc fcicq
> > [...]
> >> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> >> index 8a15511..6c5c018 100644
> >> --- a/mm/page-writeback.c
> >> +++ b/mm/page-writeback.c
> >> @@ -377,7 +377,16 @@ static unsigned long global_dirtyable_memory(void)
> >>     if (!vm_highmem_is_dirtyable)
> >>         x -= highmem_dirtyable_memory(x);
> >>
> >> -   return x + 1;   /* Ensure that we never return 0 */
> >> +   /*
> >> +    * - Why 100 ?
> >> +    * - Because the return value will be used by dirty ratio and
> >> +    *   dirty background ratio to calculate dirty thresh and bg thresh,
> >> +    *   so if the return value is two small, the thresh value maybe
> >> +    *   calculated to 0.
> >> +    *   As the max value of ratio is 100, so the return value is added
> >> +    *   by 100 here.
> >> +    */
> >> +   return x + 100;
> >
> > No. We should just revert 0f6d24f87856 ("mm/page-writeback.c: print a
> > warning if the vm dirtiness settings are illogical") because it is of a
> > dubious value and it causes problems. I am not even sure why it got
> > merged. It doesn't have any ack or review and I remember objecting to
> > the patch previously as pointless.
> > --
> 
> It is reviewed and merged by Andrew.
> 
> >From Andrew:
> I think this means that a script which alters both dirty_bytes and
> dirty_background_bytes must alter dirty_background_bytes first if they
> are being decreased and must alter dirty_bytes first if they are being
> increased.  Or something like that.

I still maintain my position that we _assume_ that admin knows what he
does and many things will break if that is not the case. Now you see
that your patch is dumping pointless warnings and you are trying to add
an ugly hack to silence them. No, this is not the way we should go. Nack
to the above. Really we should simply revert the patch. It's value is
dubious at best and it causes false positives. I do not see any reason
to not do so.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>