Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 20 Mar 2024 19:32:33 +0000 | Subject | Re: [PATCH 1/2] mm: zswap: increase shrinking protection for zswap swapins only | From | Yosry Ahmed <> |
| |
On Wed, Mar 20, 2024 at 07:48:13AM -0700, Nhat Pham wrote: > On Wed, Mar 20, 2024 at 2:51 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > > > On Wed, Mar 20, 2024 at 02:08:22AM +0000, Yosry Ahmed wrote: > > > Currently, the number of protected zswap entries corresponding to an > > > lruvec are incremented every time we swapin a page. > > > > Correct. This is the primary signal that the shrinker is being too > > aggressive in moving entries to disk and should slow down...? > > Yup. Currently, there are two scenarios in which we increase zswap > protection area: > > 1. zswap lru movement - for instance, if a page for some reasons is > rotated in the LRU. When this happens, we increment the protection > size, so that the page at the tail end of the protected area does not > lose its protection because of (potentially) spurious LRU churnings.
I don't think we update the protected area during rotations anymore, only when a new entry is added to the LRU (with potential decay).
> 2. swapin - when this happens, it is a signal that the shrinker is > being too... enthusiastic in its reclaiming action. We should be more > conservative, hence the increase in protection. > > I think there's some confusion around this, because we use the > same-ish mechanism for two different events. Maybe I should have put > yet another fat comment at the callsites of zswap_folio_swapin() too > :)
I think it makes sense. The confusion was mostly around the interpretation of finding a page on disk. I was focused on pages that skip zswap, but the main intention was for pages that were written back, as I explained in my response to Johannes.
> > > > > > This happens regardless of whether or not the page originated in > > > zswap. Hence, swapins from disk will lead to increasing protection > > > on potentially stale zswap entries. Furthermore, the increased > > Hmmm my original thinking was that, had we protected the swapped-in > page back when it was still in zswap, we would have prevented this IO. > And since the pages in zswap are (at least conceptually) "warmer" than > the swapped in page, it is appropriate to increase the zswap > protection. > > In fact, I was toying with the idea to max out the protection on > swap-in to temporarily cease all zswap shrinking, but that is perhaps > overkill :)
This would be problematic for pages that skip zswap IIUC, we'd want more shrinking in this case.
> > > > shrinking protection can lead to more pages skipping zswap and going > > I think this can only happen when the protection is so strong that the > zswap pool is full, right?
Yeah that's what I had in mind.
> > In practice I have never seen this happen though. Did you observe it? > We have a fairly aggressive lru-size-based protection decaying as > well, so that should not happen...
No this was all code inspection as I mentioned :)
> > Also technically the protection only applies to the dynamic shrinker - > the capacity-driven shrinking is not affected (although it's not very > effective - perhaps we can rework this?)
That logic is flawed anyway imo due to the acceptance threshold. We should really get rid of that as we discussed before.
| ![\](/images/icornerr.gif) |