On Fri, Jan 5, 2024 at 6:10 AM Zhongkun He <hezhongkun.hzk@xxxxxxxxxxxxx> wrote: > > > > There is another option here, which is not to move the page to the > > > tail of the inactive > > > list after end_writeback and delete the following code in > > > zswap_writeback_entry(), > > > which did not work properly. But the pages will not be released first. > > > > > > /* move it to the tail of the inactive list after end_writeback */ > > > SetPageReclaim(page); Ok, so I took a look at the patch that originally introduced this piece of logic: https://github.com/torvalds/linux/commit/b349acc76b7f65400b85abd09a5379ddd6fa5a97 Looks like it's not for the sake of correctness, but only as a best-effort optimization (reducing page scanning). If it doesn't bring any benefit (i.e due to the newly allocated page still on the cpu batch), then we can consider removing it. After all, if you're right and it's not really doing anything here - why bother. Perhaps we can replace this with some other mechanism to avoid it being scanned for reclaim. I would cc Weijie as well, as he is the original author of this.
