On Mon 03-08-20 22:18:52, Yafang Shao wrote:

On Mon, Aug 3, 2020 at 9:56 PM Michal Hocko [off-list ref] wrote:

quoted

On Mon 03-08-20 21:20:44, Yafang Shao wrote:

quoted

On Mon, Aug 3, 2020 at 6:12 PM Michal Hocko [off-list ref] wrote:

quoted

On Fri 31-07-20 09:50:04, Yafang Shao wrote:

quoted

On Thu, Jul 30, 2020 at 7:26 PM Michal Hocko [off-list ref] wrote:

quoted

On Tue 28-07-20 03:40:32, Yafang Shao wrote:

quoted

Sometimes we use memory.force_empty to drop pages in a memcg to work
around some memory pressure issues. When we use force_empty, we want the
pages can be reclaimed ASAP, however force_empty reclaims pages as a
regular reclaimer which scans the page cache LRUs from DEF_PRIORITY
priority and finally it will drop to 0 to do full scan. That is a waste
of time, we'd better do full scan initially in force_empty.

Do you have any numbers please?

Unfortunately the number doesn't improve obviously, while it is
directly proportional to the numbers of total pages to be scanned.

Your changelog claims an optimization and that should be backed by some
numbers. It is true that reclaim at a higher priority behaves slightly
and subtly differently but that urge for even more details in the
changelog.

With the below addition change (nr_to_scan also changed), the elapsed
time of force_empty can be reduced by 10%.

@@ -3208,6 +3211,7 @@ static inline bool memcg_has_children(struct

mem_cgroup *memcg)
 static int mem_cgroup_force_empty(struct mem_cgroup *memcg)
 {
        int nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
+       unsigned long size;

        /* we call try-to-free pages for make this cgroup empty */
        lru_add_drain_all();

@@ -3215,14 +3219,15 @@ static int mem_cgroup_force_empty(struct

mem_cgroup *memcg)
        drain_all_stock(memcg);
        /* try to free all pages in this cgroup */
-       while (nr_retries && page_counter_read(&memcg->memory)) {
+       while (nr_retries && (size = page_counter_read(&memcg->memory))) {
                int progress;

                if (signal_pending(current))
                        return -EINTR;
-               progress = try_to_free_mem_cgroup_pages(memcg, 1,
-                                                       GFP_KERNEL, true);
+               progress = try_to_free_mem_cgroup_pages(memcg, size,
+                                                       GFP_KERNEL, true,
+                                                       0);

Have you tried this change without changing the reclaim priority?

I tried it again. Seems the improvement is mostly due to the change of
nr_to_reclaim, rather the reclaim priority,

-               progress = try_to_free_mem_cgroup_pages(memcg, 1,
+               progress = try_to_free_mem_cgroup_pages(memcg, size,

This is what I've expected. The reclaim priority might have some side
effects as well but that requires very specific conditions when the
reclaim really has to dive to large scan windows to make some progress.
It would be interesting to find out where the improvement comes from
and how stable those numbers are. Because normally it shouldn't matter
much whether you make N rounds over the reclaim with a smaller target
or do the reclaim in a single round.
-- 
Michal Hocko
SUSE Labs