On Tue, 18 Apr 2017, Tetsuo Handa wrote: > > I have a couple of suggestions for Tetsuo about this patch, though: > > > > - We now have show_mem_rs, stall_rs, and nopage_rs. Ugh. I think it's > > better to get rid of show_mem_rs and let warn_alloc_common() not > > enforce any ratelimiting at all and leave it to the callers. > > Commit aa187507ef8bb317 ("mm: throttle show_mem() from warn_alloc()") says > that show_mem_rs was added because a big part of the output is show_mem() > which can generate a lot of output even on a small machines. Thus, I think > ratelimiting at warn_alloc_common() makes sense for users who want to use > warn_alloc_stall() for reporting stalls. > The suggestion is to eliminate show_mem_rs, it has an interval of HZ and burst of 1 when the calling function(s), warn_alloc() and warn_alloc_stall(), will have intervals of 5 * HZ and burst of 10. We don't need show_mem_rs :) > > - warn_alloc() is probably better off renamed to warn_alloc_failed() > > since it enforces __GFP_NOWARN and uses an allocation failure ratelimit > > regardless of what the passed text is. > > I'm OK to rename warn_alloc() back to warn_alloc_failed() for reporting > allocation failures. Maybe we can remove debug_guardpage_minorder() > 0 > check from warn_alloc_failed() anyway. > s/warn_alloc/warn_alloc_failed/ makes sense because the function is warning of allocation failures, not warning of allocations lol. I think the debug_guardpage_minorder() check makes sense for failed allocations because we are essentially removing memory from the system for debug, failed allocations as a result of low on memory or fragmentation aren't concerning if we are removing memory from the system. It isn't needed for allocation stalls, though, if the guard pages were actually mapped memory in use we would still be concerned about lengthy allocation stalls. So I think we should have a debug_guardpage_minorder() check for warn_alloc_failed() and not for warn_alloc_stall(). If you choose to follow-up with this, I'd happily ack it. -- 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>