Thanks for your comments. 2024年7月26日(金) 12:21 Chengming Zhou <chengming.zhou@xxxxxxxxx>: > > and, the reasons to (not) increment the progress: > > > > @@ -1387,10 +1407,20 @@ static void shrink_worker(struct work_struct *w) > > /* drop the extra reference */ > > mem_cgroup_put(memcg); > > > > - if (ret == -EINVAL) > > - break; > > + /* > > + * There are no writeback-candidate pages in the memcg. > > + * This is not an issue as long as we can find another memcg > > + * with pages in zswap. Skip this without incrementing progress > > + * and failures. > > + */ > > + if (ret == -ENOENT) > > + continue; > > + > > if (ret && ++failures == MAX_RECLAIM_RETRIES) > > break; > > + > > + /* completed writeback or incremented failures */ > > + ++progress; > > Maybe the name "progress" is a little confusing here? "progress" sounds > to me that we have some writeback completed. > > But actually it just means we have encountered some candidates, right? > > Thanks. > > Yes, the `++progress` counts both error and success as an iteration progress for valid memcgs (not writeback amount). Incrementing only on success will overly increment failures counter if there is only one memcg, one from writeback failure and one from tree walk ends, the worker aborts on 8 failures instead of 16. `++candidates;` would be better? replacing the name and fixing commit messages for v4.
