On Mon 14-09-20 23:02:15, Chunxin Zang wrote: > On Mon, Sep 14, 2020 at 9:47 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > On Mon 14-09-20 21:25:59, Chunxin Zang wrote: > > > On Mon, Sep 14, 2020 at 5:30 PM Michal Hocko <mhocko@xxxxxxxx> wrote: > > > > > > > The subject is misleading because this patch doesn't fix an infinite > > > > loop, right? It just allows the userspace to interrupt the operation. > > > > > > > > > > > Yes, so we are making a separate patch follow Vlastimil's > > recommendations. > > > Use double of threshold to end the loop. > > > > That still means the changelog needs an update > > > > The patch is already merged in Linux-next branch. Can I update the > changelog now? Yes. Andrew will refresh it. He doesn't have a git tree which would prevent rewriting the patch. > This is my first patch, please forgive me :) No worries. The mm patch workflow is rather different from others. > > > On Thu, Sep 10, 2020 at 1:59 AM Vlastimil Babka <vbabka@xxxxxxx> wrote: > > > > > From: Chunxin Zang <zangchunxin@xxxxxxxxxxxxx> > > > > > > > > > ... > > > > - IMHO it's still worth to bail out in your scenario even without a > > > > signal, e.g. > > > > by the doubling of threshold. But it can be a separate patch. > > > > Thanks! > > > > ... > > > > > > > > > > > > On Wed 09-09-20 23:20:47, zangchunxin@xxxxxxxxxxxxx wrote: > > > > > From: Chunxin Zang <zangchunxin@xxxxxxxxxxxxx> > > > > > > > > > > On our server, there are about 10k memcg in one machine. They use > > memory > > > > > very frequently. When I tigger drop caches，the process will infinite > > loop > > > > > in drop_slab_node. > > > > > > > > Is this really an infinite loop, or it just takes a lot of time to > > > > process all the metadata in that setup? If this is really an infinite > > > > loop then we should look at it. My current understanding is that the > > > > operation would finish at some time it just takes painfully long to get > > > > there. > > > > > > > > > > Yes, it's really an infinite loop. Every loop spends a lot of time. In > > > this time, > > > memcg will alloc/free memory, so the next loop, the total of 'freed' > > > always bigger than 10. > > > > I am still not sure I follow. Do you mean that there is somebody > > constantly generating more objects to reclaim? > > > > Yes, this is my meaning. :) > > > > Maybe we are just not agreeing on the definition of an infinite loop but > > in my book that means that the final condition can never be met. While a > > busy adding new object might indeed cause drop caches to loop for a long > > time this is to be expected from that interface as it is supposed to > > drop all the cache and that can grow during the operation. > > -- > > > > Because I have 10k memcg , all of them are heavy users of memory. > During each loop, there are always more than 10 reclaimable objects > generating, so the > condition is never met. 10k or any number of memcgs shouldn't really make much of a difference. Except for the time the scan adds. Fundamentally we are talking about freed objects and whether they are on the global or per memcg lists should result in a similar behavior. > The drop cache process has no chance to exit the > loop. > Although the purpose of the 'drop cache' interface is to release all > caches, we still need a > way to terminate it, e.g. in this case, the process took too long to run . Yes, this is perfectly understandable. Having a bail out on fatal signal is a completely reasonable thing to do. I am mostly confused by your infinite loop claims and what the relation of this patch to it. I would propose this wording instead " We have observed that drop_caches can take a considerable amount of time (<put data here>). Especially when there are many memcgs involved because they are adding an additional overhead. It is quite unfortunate that the operation cannot be interrupted by a signal currently. Add a check for fatal signals into the main loop so that userspace can control early bailout. " or something along those lines. > > root 357956 ... R Aug25 21119854:55 echo 3 > /proc/sys/vm/drop_caches -- Michal Hocko SUSE Labs
