Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1503285

Re: [PATCH v5] z3fold: add shrinker

Show all headers | View raw

On Tue, Oct 18, 2016 at 7:35 PM, Dan Streetman <ddstreet@ieee.org> wrote:
> On Tue, Oct 18, 2016 at 12:26 PM, Vitaly Wool <vitalywool@gmail.com> wrote:
>> 18 окт. 2016 г. 18:29 пользователь "Dan Streetman" <ddstreet@ieee.org>
>> написал:
>>
>>
>>>
>>> On Tue, Oct 18, 2016 at 10:51 AM, Vitaly Wool <vitalywool@gmail.com>
>>> wrote:
>>> > On Tue, Oct 18, 2016 at 4:27 PM, Dan Streetman <ddstreet@ieee.org>
>>> > wrote:
>>> >> On Mon, Oct 17, 2016 at 10:45 PM, Vitaly Wool <vitalywool@gmail.com>
>>> >> wrote:
>>> >>> Hi Dan,
>>> >>>
>>> >>> On Tue, Oct 18, 2016 at 4:06 AM, Dan Streetman <ddstreet@ieee.org>
>>> >>> wrote:
>>> >>>> On Sat, Oct 15, 2016 at 8:05 AM, Vitaly Wool <vitalywool@gmail.com>
>>> >>>> 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()?
>>
>> That would give a huge performance penalty (checked).
>
> my core concern with the shrinker is, it has no context about which
> pages need compacting and which don't, while the alloc/free/unmap
> functions do.  If the alloc/free/unmap fast paths are impacted too
> much by compacting directly, then yeah setting a flag for deferred
> action would be better than the shrinker just scanning all pages.
> However, in that the case, then a shrinker still seems unnecessary -
> all the pages that need compacting are pre-marked, there's no need to
> scan any more.  Isn't a simple workqueue to deferred-compact pages
> better?

Well yep, the more I think of that the more it seems a better fit. I'll test
that workqueue thing performance wise and get back with a new patch.

~vitaly

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v5] z3fold: add shrinker Vitaly Wool <vitalywool@gmail.com> - 2016-10-15 14:00 +0200
  [PATCH v5 3/3] z3fold: add shrinker Vitaly Wool <vitalywool@gmail.com> - 2016-10-15 14:10 +0200
    Re: [PATCH v5 3/3] z3fold: add shrinker Dan Streetman <ddstreet@ieee.org> - 2016-10-18 04:10 +0200
      Re: [PATCH v5 3/3] z3fold: add shrinker Vitaly Wool <vitalywool@gmail.com> - 2016-10-18 04:50 +0200
        Re: [PATCH v5 3/3] z3fold: add shrinker Dan Streetman <ddstreet@ieee.org> - 2016-10-18 16:30 +0200
          Re: [PATCH v5 3/3] z3fold: add shrinker Vitaly Wool <vitalywool@gmail.com> - 2016-10-18 17:00 +0200
            Re: [PATCH v5 3/3] z3fold: add shrinker Dan Streetman <ddstreet@ieee.org> - 2016-10-18 17:40 +0200
              Re: [PATCH v5] z3fold: add shrinker Dan Streetman <ddstreet@ieee.org> - 2016-10-18 19:40 +0200
                Re: [PATCH v5] z3fold: add shrinker Vitaly Wool <vitalywool@gmail.com> - 2016-10-18 20:40 +0200
  [PATCH v5 2/3] z3fold: remove redundant locking Vitaly Wool <vitalywool@gmail.com> - 2016-10-15 14:20 +0200
    Re: [PATCH v5 2/3] z3fold: remove redundant locking Dan Streetman <ddstreet@ieee.org> - 2016-10-17 23:00 +0200
      Re: [PATCH v5 2/3] z3fold: remove redundant locking Vitaly Wool <vitalywool@gmail.com> - 2016-10-18 05:00 +0200

csiph-web