2017-11-28 15:52 GMT+08:00 Yafang Shao <laoar.shao@xxxxxxxxx>: > 2017-11-28 15:45 GMT+08:00 Michal Hocko <mhocko@xxxxxxxx>: >> On Tue 28-11-17 14:12:15, Yafang Shao wrote: >>> 2017-11-28 11:11 GMT+08:00 Yafang Shao <laoar.shao@xxxxxxxxx>: >>> > Hi Michal, >>> > >>> > What about bellow change ? >>> > It makes the function domain_dirty_limits() more clear. >>> > And the result will have a higher precision. >>> > >>> > >>> > diff --git a/mm/page-writeback.c b/mm/page-writeback.c >>> > index 8a15511..2b5e507 100644 >>> > --- a/mm/page-writeback.c >>> > +++ b/mm/page-writeback.c >>> > @@ -397,8 +397,8 @@ static void domain_dirty_limits(struct >>> > dirty_throttle_control *dtc) >>> > unsigned long bytes = vm_dirty_bytes; >>> > unsigned long bg_bytes = dirty_background_bytes; >>> > /* convert ratios to per-PAGE_SIZE for higher precision */ >>> > - unsigned long ratio = (vm_dirty_ratio * PAGE_SIZE) / 100; >>> > - unsigned long bg_ratio = (dirty_background_ratio * PAGE_SIZE) / 100; >>> > + unsigned long ratio = vm_dirty_ratio; >>> > + unsigned long bg_ratio = dirty_background_ratio; >>> > unsigned long thresh; >>> > unsigned long bg_thresh; >>> > struct task_struct *tsk; >>> > @@ -416,28 +416,33 @@ static void domain_dirty_limits(struct >>> > dirty_throttle_control *dtc) >>> > */ >>> > if (bytes) >>> > ratio = min(DIV_ROUND_UP(bytes, global_avail), >>> > - PAGE_SIZE); >>> > + 100); >>> > if (bg_bytes) >>> > bg_ratio = min(DIV_ROUND_UP(bg_bytes, global_avail), >>> > - PAGE_SIZE); >>> > + 99); /* bg_ratio should less than ratio */ >>> > bytes = bg_bytes = 0; >>> > } >>> >>> >>> Errata: >>> >>> if (bytes) >>> - ratio = min(DIV_ROUND_UP(bytes, global_avail), >>> - PAGE_SIZE); >>> + ratio = min(DIV_ROUND_UP(bytes / PAGE_SIZE, global_avail), >>> + 100); >>> if (bg_bytes) >>> - bg_ratio = min(DIV_ROUND_UP(bg_bytes, global_avail), >>> - PAGE_SIZE); >>> + bg_ratio = min(DIV_ROUND_UP(bg_bytes / PAGE_SIZE, global_avail), >>> + 100 - 1); /* bg_ratio should be less than ratio */ >>> bytes = bg_bytes = 0; >> >> And you really think this makes code easier to follow? I am somehow not >> conviced... >> > > There's hidden bug in the original code, because it is too complex to > clearly understand. > See bellow, > > ratio = min(DIV_ROUND_UP(bytes, global_avail), > PAGE_SIZE) > > Suppose the vm_dirty_bytes is set to 512M (this is a reasonable > value), and the global_avail is only 10000 pages (this is not low), > then DIV_ROUND_UP(bytes, global_avail) is 53688, which is bigger than > 4096, so the ratio will be 4096. > That's unreasonable. > Besides, when gdtc is NULL(meaning not for memcg), bg_thresh and thresh could both be bigger than available_memory when available_memory is very low. So what is your opinion on that confused code ? My opinion is when available_memory is very low, don't wake up for_background writeback, just let the for_kupdate writeback flush the dirty data. Thanks Yafang -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
