Yosry Ahmed <yosryahmed@xxxxxxxxxx> writes: > On Tue, Mar 28, 2023 at 6:33 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote: >> >> Yosry Ahmed <yosryahmed@xxxxxxxxxx> writes: >> >> > On Tue, Mar 28, 2023 at 2:32 PM Chris Li <chrisl@xxxxxxxxxx> 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@xxxxxxxxxx> 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. 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? >
