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@xxxxxxxxxxx> 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.
