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. > > The swap entry space is smaller than the memory address space and > there are other swapfiles can use the swap entry. You do need to do > some swap entry space management work to get a free entry. That to > me seems unavoidable, am I missing something? > > > > Personally, I have no problem to change the design of swap code to add > > > useful features. Just want to check whether we can do that step by step > > > and show benefit and cost clearly in each step. > > > > Right. I understand and totally agree, even from a development point > > of view it's much better to make big changes incrementally to avoid > > doing a lot of work that ends up going nowhere. I am just trying to > > make sure that whatever we decide is indeed a step in the right > > direction. > > The ghost swap file patch already exists. We might just share it here > for the discussion purpose, list the pros and cons. The ghost swap file patch that we have does not work upstream because: (a) It involves ABI changes (we start supporting swapon on a sparse 0-block-length file). Hence, it cannot be an incremental/intermediate step because once we start supporting it we cannot take it back. (b) It requires modifications to the swapon utility to work. In my response to Johannes [1] I described an alternative that can work upstream and wouldn't have ABI changes or need changes to swapon. [1]https://lore.kernel.org/linux-mm/CAJD7tkbudmPTEumgKJZ5pXy6O79ySbGiCnAZXnUUuEmfZ6KCtQ@xxxxxxxxxxxxxx/ > > Chris >
