Messages in this thread | | | Date | Wed, 28 Jan 2015 16:14:04 +0900 | From | Heesub Shin <> | Subject | Re: [RFC PATCH 0/9] mm/zbud: support highmem pages |
| |
On 01/28/2015 05:24 AM, Seth Jennings wrote: > On Tue, Nov 04, 2014 at 10:33:43AM -0600, Seth Jennings wrote: >> On Tue, Oct 14, 2014 at 08:59:19PM +0900, Heesub Shin wrote: >>> zbud is a memory allocator for storing compressed data pages. It keeps >>> two data objects of arbitrary size on a single page. This simple design >>> provides very deterministic behavior on reclamation, which is one of >>> reasons why zswap selected zbud as a default allocator over zsmalloc. >>> >>> Unlike zsmalloc, however, zbud does not support highmem. This is >>> problomatic especially on 32-bit machines having relatively small >>> lowmem. Compressing anonymous pages from highmem and storing them into >>> lowmem could eat up lowmem spaces. >>> >>> This limitation is due to the fact that zbud manages its internal data >>> structures on zbud_header which is kept in the head of zbud_page. For >>> example, zbud_pages are tracked by several lists and have some status >>> information, which are being referenced at any time by the kernel. Thus, >>> zbud_pages should be allocated on a memory region directly mapped, >>> lowmem. >>> >>> After some digging out, I found that internal data structures of zbud >>> can be kept in the struct page, the same way as zsmalloc does. So, this >>> series moves out all fields in zbud_header to struct page. Though it >>> alters quite a lot, it does not add any functional differences except >>> highmem support. I am afraid that this kind of modification abusing >>> several fields in struct page would be ok. >> >> Hi Heesub, >> >> Sorry for the very late reply. The end of October was very busy for me. >> >> A little history on zbud. I didn't put the metadata in the struct >> page, even though I knew that was an option since we had done it with >> zsmalloc. At the time, Andrew Morton had concerns about memmap walkers >> getting messed up with unexpected values in the struct page fields. In >> order to smooth zbud's acceptance, I decided to store the metadata >> inline in the page itself. >> >> Later, zsmalloc eventually got accepted, which basically gave the >> impression that putting the metadata in the struct page was acceptable. >> >> I have recently been looking at implementing compaction for zsmalloc, >> but having the metadata in the struct page and having the handle >> directly encode the PFN and offset of the data block prevents >> transparent relocation of the data. zbud has a similar issue as it >> currently encodes the page address in the handle returned to the user >> (also the limitation that is preventing use of highmem pages). >> >> I would like to implement compaction for zbud too and moving the >> metadata into the struct page is going to work against that. In fact, >> I'm looking at the option of converting the current zbud_header into a >> per-allocation metadata structure, which would provide a layer of >> indirection between zbud and the user, allowing for transparent >> relocation and compaction. > > I had some downtime and started thinking about this again today (after > 3 months). > > Upon further reflection, I really like this and don't think that it > inhibits introducing compaction later. > > There are just a few places that look messy or problematic to me: > > 1. the use of page->private and masking the number of chunks for both > buddies into it (see suggestion for overlay struct below) > 2. the use of the second double word &page->index to store a list_head > > #2 might be problematic because, IIRC, memmap walkers will check _count > (or _mapcount). I think we ran into this in zsmalloc. > > Initially, when working on zsmalloc, I just created a structure that > overlaid the struct page in the memmap, reserving the flags and _count > areas, so that I wouldn't have to be bound by the field names/boundaries > in the struct page. > > IIRC, Andrew was initially against that, but he was also against the > whole idea of using the struct page fields for random stuff... I that > ended up being accepted. > > This code looks really good! I think with a little cleanup and finding > a way to steer clear of using the _count part of the structure, this > will be great.
Thanks for your comments! I will try to address problems you pointed and post a new patchset hopefully soon.
regards, heesub
> > Sorry for dismissing it earlier. Didn't give it enough credit. > > Thanks, > Seth > >> >> However, I do like the part about letting zbud use highmem pages. >> >> I have something in mind that would allow highmem pages _and_ move >> toward something that would support compaction. I'll see if I can put >> it into code today. >> >> Thanks, >> Seth >> >>> >>> Heesub Shin (9): >>> mm/zbud: tidy up a bit >>> mm/zbud: remove buddied list from zbud_pool >>> mm/zbud: remove lru from zbud_header >>> mm/zbud: remove first|last_chunks from zbud_header >>> mm/zbud: encode zbud handle using struct page >>> mm/zbud: remove list_head for buddied list from zbud_header >>> mm/zbud: drop zbud_header >>> mm/zbud: allow clients to use highmem pages >>> mm/zswap: use highmem pages for compressed pool >>> >>> mm/zbud.c | 244 ++++++++++++++++++++++++++++++------------------------------- >>> mm/zswap.c | 4 +- >>> 2 files changed, 121 insertions(+), 127 deletions(-) >>> >>> -- >>> 1.9.1 >>> >>> -- >>> To unsubscribe, send a message with 'unsubscribe linux-mm' in >>> the body to majordomo@kvack.org. For more info on Linux MM, >>> see: http://www.linux-mm.org/ . >>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >> >> -- >> To unsubscribe, send a message with 'unsubscribe linux-mm' in >> the body to majordomo@kvack.org. For more info on Linux MM, >> see: http://www.linux-mm.org/ . >> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> >
| |