On Tue, Mar 28, 2023 at 12:01 AM Huang, Ying <ying.huang@xxxxxxxxx> wrote: > > Yosry Ahmed <yosryahmed@xxxxxxxxxx> writes: > > > On Mon, Mar 27, 2023 at 11:22 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote: > >> > >> Yosry Ahmed <yosryahmed@xxxxxxxxxx> writes: > >> > >> > On Sun, Mar 26, 2023 at 6:24 PM Huang, Ying <ying.huang@xxxxxxxxx> wrote: > >> >> > >> >> Chris Li <chrisl@xxxxxxxxxx> writes: > >> >> > >> >> > On Fri, Mar 24, 2023 at 12:28:31AM -0700, Yosry Ahmed wrote: > >> >> >> > In fact, I just suggest to use the minimal design on top of the current > >> >> >> > implementation as the first step. Then, you can improve it step by > >> >> >> > step. > >> >> >> > > >> >> >> > The first step could be the minimal effort to implement indirection > >> >> >> > layer and moving swapped pages between swap implementations. Based on > >> >> >> > that, you can build other optimizations, such as pulling swap counting > >> >> >> > to the swap core. For each step, we can evaluate the gain and cost with > >> >> >> > data. > >> >> >> > >> >> >> Right, I understand that, but to implement the indirection layer on > >> >> >> top of the current implementation, then we will need to support using > >> >> >> zswap without a backing swap device. In order to do this without > >> >> > > >> >> > Agree with Ying on the minimal approach here as well. > >> >> > > >> >> > There are two ways to approach this. > >> >> > > >> >> > 1) Forget zswap, make a minimal implementation to move the page between > >> >> > two swapfile device. It can be swapfile back to two loop back files. > >> >> > > >> >> > Any indirect layer you design will need to convert this usage case > >> >> > any way. > >> >> > > >> >> > 2) Make zswap work without a swapfile. > >> >> > You can implement the zswap on a fake ghosts swap file. > >> >> > > >> >> > If you keep the zswap as frontswap, just make zswap can work without > >> >> > a real swapfile. > >> >> > > >> >> > Make that as your first minimal step. Then it does not need to touch > >> >> > the swap count changes. > >> >> > > >> >> > I view make that step is independent of moving pages between swap device. > >> >> > > >> >> > That patch exists and I consider it has value to some users. > >> >> > >> >> This sounds like an even smaller approach as the first step. Further > >> >> improvement can be built on top of it. > >> > > >> > I am not sure how this would be a step towards the abstraction goal we > >> > have been discussing. > >> > > >> > We have been discussing starting out with a minimal indirection layer, > >> > in the shape of an xarray that maps a swap ID to a swap entry, and > >> > that can be disabled with a config option. > >> > > >> > For such a design to work, we have to implement swap entry management > >> > & swap counting in zswap, right? Am I missing something? > >> > >> Chris suggested to avoid to implement the swap entry management & swap > >> counting in zswap via using a "fake ghost swap file". Copied his > >> suggestion as below, > > > > Right, we have been using ghost swapfiles at Google for a while. They > > are basically sparse files that you can never actually write to, they > > are just used so that we can use zswap without a backing swap device. > > > > What I do not understand is how this is a step towards the ultimate > > goal of swap abstraction. Is the idea to have the indirection layer > > only support moving swapped pages between swapfiles, and have those > > "ghost" swapfiles be on a higher tier than normal swapfiles? In this > > case, I am guessing we eliminate the writeback logic from zswap itself > > and move it to this indirection layer. > > Yes. I think the suggested minimal first step includes replacing the > writeback logic of zswap itself with moving swapped page of swap core > (indirectly layer). > > > 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? 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. > > > We also have to unnecessarily limit the size of zswap with the size of > > this fake swapfile. > > I guess you need to limit the size of zswap anyway, because you need to > decide when to start to writeback or moving to the lower tiers. zswap has a knob to limit its size, but based on the actual memory usage of zswap (i.e the size of compressed pages). There is ongoing work as well to autotune this if I remember correctly. Having to deal with both the limit on compressed memory and the limited on the uncompressed size of swapped pages is cumbersome. Again, we already have this behavior today, but the initial swap_desc proposal aimed to avoid it. > > > In other words, we retain a lot of limitations that we have today. > > As the minimal first step, not the final state. I am assuming the first step here is using ghost swapfiles to support zswap without a backing swap device and having an indirection layer between swapfiles. A following step can be making this indirection layer support zswap directly without having to use such a ghost swapfile, if the cost-benefit analysis proves that this is worthwhile. Is my understanding correct? > > > Keep in mind that supporting ghost swapfiles is something that > > is exposed to userspace, so we have to commit to supporting it -- it > > can't just be an incremental step that we will change later. > > Yes. We should really care about ABI. It's not a good idea to add ABI > for an intermediate step. Do we need to change ABI to use a sparse file > to backing zswap? I think so. At Google we identify ghost swapfiles by having 0 block length and mark them in the kernel. The upstream kernel would reject such a file because it doesn't have a proper swap header. If we use mkswap to have a proper swap header then swapon rejects the file because it has holes. > > > With all that said, it is certainly a much simpler "solution". > > Interested to hear thoughts on this, we can certainly pursue it if > > people think it is the right way to move forward. > > 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. Thanks for a very insightful discussion. > > Best Regards, > Huang, Ying > > >> > >> " > >> >> > 2) Make zswap work without a swapfile. > >> >> > You can implement the zswap on a fake ghosts swap file. > >> >> > > >> >> > If you keep the zswap as frontswap, just make zswap can work without > >> >> > a real swapfile. > >> >> > > >> >> > Make that as your first minimal step. Then it does not need to touch > >> >> > the swap count changes. > >> " > >> > >> Best Regards, > >> Huang, Ying > >> > >> >> > >> >> >> > Anyway, I don't think you can just implement all your final solution in > >> >> >> > one step. And, I think the minimal design suggested could be a starting > >> >> >> > point. > >> >> >> > >> >> >> I agree that's a great point, I am just afraid that we will avoid > >> >> >> implementing that full final solution and instead do a lot of work > >> >> >> inside zswap to make up for the difference (e.g. swap entry > >> >> >> management, swap counting). Also, that work in zswap may end up being > >> >> >> unacceptable due to the maintenance burden and/or complexity. > >> >> > > >> >> > If you do either 1) or 2), you can keep these two paths separate. > >> >> > > >> >> > Even if you want to move the page between zswap and swapfile. > >> >> > > >> >> > Idea 3) > >> >> > You don't have to change the swap count code, you can do a > >> >> > minimal change moves the page between zswap and another block > >> >> > device. That way you can get two differenet swap entry with > >> >> > existing code. > >> >> > > >> >> > Chris > >> >> > >> >
