On Tue, Oct 18, 2016 at 10:51 AM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: > On Tue, Oct 18, 2016 at 4:27 PM, Dan Streetman <ddstreet@xxxxxxxx> wrote: >> On Mon, Oct 17, 2016 at 10:45 PM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: >>> Hi Dan, >>> >>> On Tue, Oct 18, 2016 at 4:06 AM, Dan Streetman <ddstreet@xxxxxxxx> wrote: >>>> On Sat, Oct 15, 2016 at 8:05 AM, Vitaly Wool <vitalywool@xxxxxxxxx> wrote: >>>>> This patch implements shrinker for z3fold. This shrinker >>>>> implementation does not free up any pages directly but it allows >>>>> for a denser placement of compressed objects which results in >>>>> less actual pages consumed and higher compression ratio therefore. >>>>> >>>>> This update removes z3fold page compaction from the freeing path >>>>> since we can rely on shrinker to do the job. Also, a new flag >>>>> UNDER_COMPACTION is introduced to protect against two threads >>>>> trying to compact the same page. >>>> >>>> i'm completely unconvinced that this should be a shrinker. The >>>> alloc/free paths are much, much better suited to compacting a page >>>> than a shrinker that must scan through all the unbuddied pages. Why >>>> not just improve compaction for the alloc/free paths? >>> >>> Basically the main reason is performance, I want to avoid compaction on hot >>> paths as much as possible. This patchset brings both performance and >>> compression ratio gain, I'm not sure how to achieve that with improving >>> compaction on alloc/free paths. >> >> It seems like a tradeoff of slight improvement in hot paths, for >> significant decrease in performance by adding a shrinker, which will >> do a lot of unnecessary scanning. The alloc/free/unmap functions are >> working directly with the page at exactly the point where compaction >> is needed - when adding or removing a bud from the page. > > I can see that sometimes there are substantial amounts of pages that > are non-compactable synchronously due to the MIDDLE_CHUNK_MAPPED > bit set. Picking up those seems to be a good job for a shrinker, and those > end up in the beginning of respective unbuddied lists, so the shrinker is set > to find them. I can slightly optimize that by introducing a > COMPACT_DEFERRED flag or something like that to make shrinker find > those pages faster, would that make sense to you? Why not just compact the page in z3fold_unmap()? > >> Sorry if I missed it in earlier emails, but have you done any >> performance measurements comparing with/without the shrinker? The >> compression ratio gains may be possible with only the >> z3fold_compact_page() improvements, and performance may be stable (or >> better) with only a per-z3fold-page lock, instead of adding the >> shrinker...? > > I'm running some tests with per-page locks now, but according to the > previous measurements the shrinker version always wins on multi-core > platforms. But that comparison is without taking the spinlock in map/unmap right? > >> If a shrinker really is needed, it seems like it would be better >> suited to coalescing separate z3fold pages via migration, like >> zsmalloc does (although that's a significant amount of work). > > I really don't want to go that way to keep z3fold applicable to an MMU-less > system. > > ~vitaly -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>