Re: [LSF/MM/BPF TOPIC] Swap Abstraction / Native Zswap

["Huang, Ying" <ying.huang@intel.com>]

Yosry Ahmed <yosryahmed@google.com> writes:

> On Tue, Apr 4, 2023 at 1:12 AM Huang, Ying <ying.huang@intel.com> wrote:
>>
>> Yosry Ahmed <yosryahmed@google.com> writes:
>>
>> > On Tue, Mar 28, 2023 at 6:33 PM Huang, Ying <ying.huang@intel.com> wrote:
>> >>
>> >> Yosry Ahmed <yosryahmed@google.com> writes:
>> >>
>> >> > On Tue, Mar 28, 2023 at 2:32 PM Chris Li <chrisl@kernel.org> wrote:
>> >> >>
>> >> >> On Tue, Mar 28, 2023 at 02:01:09PM -0700, Yosry Ahmed wrote:
>> >> >> > On Tue, Mar 28, 2023 at 1:50 PM Chris Li <chrisl@kernel.org> wrote:
>> >> >> > >
>> >> >> > > On Tue, Mar 28, 2023 at 12:59:31AM -0700, Yosry Ahmed wrote:
>> >> >> > > > > > I don't have a problem with this approach, it is not really clean as
>> >> >> > > > > > we still treat zswap as a swapfile and have to deal with a lot of
>> >> >> > > > > > unnecessary code like swap slots handling and whatnot.
>> >> >> > > > >
>> >> >> > > > > These are existing code?
>> >> >> > >
>> >> >> > > Yes. The ghost swap file are existing code used in Google for many years.
>> >> >> > >
>> >> >> > > > I was referring to the fact that today with zswap being tied to
>> >> >> > > > swapfiles we do some necessary work such as searching for swap slots
>> >> >> > > > during swapout. The initial swap_desc approach aimed to avoid that.
>> >> >> > > > With this minimal ghost swapfile approach we retain this unfavorable
>> >> >> > > > behavior.
>> >> >> > >
>> >> >> > > Can you explain how you can avoid the free swap entry search
>> >> >> > > in the swap descriptor world?
>> >> >> >
>> >> >> > For zswap, in the swap descriptor world, you just need to allocate a
>> >> >> > struct zswap_entry and have the swap descriptor point to it. No need
>> >> >> > for swap slot management since we are not tied to a swapfile and pages
>> >> >> > in zswap do not have a specific position.
>> >> >>
>> >> >> Your swap descriptor will be using one swp_entry_t, which get from the PTE
>> >> >> to lookup, right? That is the swap entry I am talking about. You just
>> >> >> substitute zswap swap entry with the swap descriptor swap entry.
>> >> >> You still need to allocate from the free swap entry space at least once.
>> >> >
>> >> > Oh, you mean the swap ID space. We just need to find an unused ID, we
>> >> > can simply use an allocating xarray
>> >> > (https://docs.kernel.org/core-api/xarray.html#allocating-xarrays).
>> >> > This is simpler than keeping track of swap slots in a swapfile.
>> >>
>> >> If we want to implement the swap entry management inside the zswap
>> >> implementation (instead of reusing swap_map[]), then the allocating
>> >> xarray can be used too.  Some per-entry data (such as swap count, etc.)
>> >> can be stored there.  I understanding that this isn't perfect (one more
>> >> xarray looking up, one more data structure, etc.), but this is a choice
>> >> too.
>> >
>> > My main concern here would be having two separate swap counting
>> > implementations -- although it might not be the end of the world.
>>
>> This isn't a big issue for me.  For file systems, there are duplicated
>> functionality in different file system implementation, such as free
>> block space management.  Instead, I hope we can design better swap
>> implementation in the future.
>>
>> > It would be useful to consider all the options. So far, I think we
>> > have been discussing 3 alternatives:
>> >
>> > (a) The initial swap_desc proposal.
>>
>> My main concern for the initial swap_desc proposal is that the zswap
>> code is put in swap core instead of zswap implementation per my
>> understanding.  So zswap isn't another swap implementation encapsulated
>> with a common interface.  Please correct me if my understanding isn't
>> correct.
>>
>> If so, the flexibility of the swap system is the cost.  For example,
>> zswap may be always at the highest priority among all swap devices.  We
>> can move the cold page from zswap to some swap device.  But we cannot
>> move the cold page from some swap device to zswap.
>
>
> Not really. In the swap_desc proposal, I intended to have struct
> swap_desc contain either a swap device entry (swp_entry_t) or a
> frontswap entry (a pointer). zswap implementation would not be in the
> swap core, instead, we would have two swap implementations: swap
> devices and frontswap/zswap -- each of which implement a common swap
> API. We can use one of the free bits to distinguish the type of the
> underlying entry (swp_entry_t or pointer to frontswap/zswap entry).
>
> We can start by only supporting moving pages from frontswap/zswap to
> swap devices, but I don't see why the same design would not support
> pages moving in the other direction if the need arises.
>
> The number of free bits in swp_entry_t and pointers is limited (2 bits
> on 32-bit systems, 3 bits on 64-bit systems), so there are only a
> handful of different swap types we can support with the swap_desc
> design, but we only need two to begin with. If in the future we need
> more, we can add an indirection layer then or expand swap_desc -- or
> we can encode the data within the swap device itself (how it compares
> to frontswap/zswap).
>
> In summary, the swap_desc proposal does NOT involve moving zswap code
> to core swap, it involves a generic swap API with two implementations:
> swap devices and frontswap/zswap.

This eliminate the main concerns for me!  Thanks!

> The only problems I see with the swap_desc design are:
> - Extra overhead for users using swapfiles only.
> - A bigger leap from what we have today than other ideas proposed
> (e.g. virtual swap device for zswap).

Yes.

Best Regards,
Huang, Ying

>>
>>
>> Maybe compression is always faster than any other swap devices, so we
>> will never need the flexibility.  Maybe the cost to hide zswap behind a
>> common interface is unacceptable.  I'm open to these.  But please
>> provide the evidence, and maybe data.
>>
>> Best Regards,
>> Huang, Ying
>>
>> > (b) Add an optional indirection layer that can move swap entries
>> > between swap devices and add a virtual swap device for zswap in the
>> > kernel.
>> > (c) Add an optional indirection layer that can move entries between
>> > different swap backends. Swap backends would be zswap & swap devices
>> > for now. Zswap needs to implement swap entry management, swap
>> > counting, etc.
>> >
>> > Does this accurately summarize what we have discussed so far?
>> >