All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Vovo Yang <vovoy@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>,
	linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	linux-mm@kvack.org, Chris Wilson <chris@chris-wilson.co.uk>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable
Date: Fri, 2 Nov 2018 19:26:16 +0100	[thread overview]
Message-ID: <20181102182616.GJ28039@dhcp22.suse.cz> (raw)
In-Reply-To: <CAEHM+4rvBmFWhzPXZrwxXvMEmVdkpsgRg26wVNYSA8HKF_8AwQ@mail.gmail.com>

On Fri 02-11-18 20:35:11, Vovo Yang wrote:
> On Thu, Nov 1, 2018 at 9:10 PM Michal Hocko <mhocko@kernel.org> wrote:
> > OK, so that explain my question about the test case. Even though you
> > generate a lot of page cache, the amount is still too small to trigger
> > pagecache mostly reclaim and anon LRUs are scanned as well.
> >
> > Now to the difference with the previous version which simply set the
> > UNEVICTABLE flag on mapping. Am I right assuming that pages are already
> > at LRU at the time? Is there any reason the mapping cannot have the flag
> > set before they are added to the LRU?
> 
> I checked again. When I run gem_syslatency, it sets unevictable flag
> first and then adds pages to LRU, so my explanation to the previous
> test result is wrong. It should not be necessary to explicitly move
> these pages to unevictable list for this test case.

OK, that starts to make sense finally.

> The performance
> improvement of this patch on kbl might be due to not calling
> shmem_unlock_mapping.

Yes that one can get quite expensive. find_get_entries is really
pointless here because you already do have your pages. Abstracting
check_move_unevictable_pages into a pagevec api sounds like a reasonable
compromise between the code duplication and relatively low-level api to
export.

> The perf result of a shmem lock test shows find_get_entries is the
> most expensive part of shmem_unlock_mapping.
> 85.32%--ksys_shmctl
>         shmctl_do_lock
>          --85.29%--shmem_unlock_mapping
>                    |--45.98%--find_get_entries
>                    |           --10.16%--radix_tree_next_chunk
>                    |--16.78%--check_move_unevictable_pages
>                    |--16.07%--__pagevec_release
>                    |           --15.67%--release_pages
>                    |                      --4.82%--free_unref_page_list
>                    |--4.38%--pagevec_remove_exceptionals
>                     --0.59%--_cond_resched

-- 
Michal Hocko
SUSE Labs

WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org>
To: Vovo Yang <vovoy@chromium.org>
Cc: linux-mm@kvack.org, Peter Zijlstra <peterz@infradead.org>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Dave Hansen <dave.hansen@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable
Date: Fri, 2 Nov 2018 19:26:16 +0100	[thread overview]
Message-ID: <20181102182616.GJ28039@dhcp22.suse.cz> (raw)
In-Reply-To: <CAEHM+4rvBmFWhzPXZrwxXvMEmVdkpsgRg26wVNYSA8HKF_8AwQ@mail.gmail.com>

On Fri 02-11-18 20:35:11, Vovo Yang wrote:
> On Thu, Nov 1, 2018 at 9:10 PM Michal Hocko <mhocko@kernel.org> wrote:
> > OK, so that explain my question about the test case. Even though you
> > generate a lot of page cache, the amount is still too small to trigger
> > pagecache mostly reclaim and anon LRUs are scanned as well.
> >
> > Now to the difference with the previous version which simply set the
> > UNEVICTABLE flag on mapping. Am I right assuming that pages are already
> > at LRU at the time? Is there any reason the mapping cannot have the flag
> > set before they are added to the LRU?
> 
> I checked again. When I run gem_syslatency, it sets unevictable flag
> first and then adds pages to LRU, so my explanation to the previous
> test result is wrong. It should not be necessary to explicitly move
> these pages to unevictable list for this test case.

OK, that starts to make sense finally.

> The performance
> improvement of this patch on kbl might be due to not calling
> shmem_unlock_mapping.

Yes that one can get quite expensive. find_get_entries is really
pointless here because you already do have your pages. Abstracting
check_move_unevictable_pages into a pagevec api sounds like a reasonable
compromise between the code duplication and relatively low-level api to
export.

> The perf result of a shmem lock test shows find_get_entries is the
> most expensive part of shmem_unlock_mapping.
> 85.32%--ksys_shmctl
>         shmctl_do_lock
>          --85.29%--shmem_unlock_mapping
>                    |--45.98%--find_get_entries
>                    |           --10.16%--radix_tree_next_chunk
>                    |--16.78%--check_move_unevictable_pages
>                    |--16.07%--__pagevec_release
>                    |           --15.67%--release_pages
>                    |                      --4.82%--free_unref_page_list
>                    |--4.38%--pagevec_remove_exceptionals
>                     --0.59%--_cond_resched

-- 
Michal Hocko
SUSE Labs
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-11-02 18:26 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-31  8:19 [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable Kuo-Hsin Yang
2018-10-31  8:19 ` Kuo-Hsin Yang
2018-10-31  9:41 ` Chris Wilson
2018-10-31  9:41   ` Chris Wilson
2018-10-31  9:41   ` Chris Wilson
2018-10-31 10:42   ` Vovo Yang
2018-11-01 12:20   ` Chris Wilson
2018-11-01 12:20     ` Chris Wilson
2018-11-01 12:20     ` Chris Wilson
2018-10-31 11:05 ` ✗ Fi.CI.CHECKPATCH: warning for mm, drm/i915: Mark pinned shmemfs pages as unevictable (rev2) Patchwork
2018-10-31 11:22 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-31 14:19 ` [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable Dave Hansen
2018-10-31 14:19   ` Dave Hansen
2018-11-01 12:06   ` Vovo Yang
2018-11-01 12:06     ` Vovo Yang
2018-11-01 14:30     ` Dave Hansen
2018-11-02 13:22       ` Vovo Yang
2018-11-02 14:05         ` Dave Hansen
2018-11-05 11:24           ` Kuo-Hsin Yang
2018-10-31 14:24 ` Michal Hocko
2018-10-31 14:40   ` Dave Hansen
2018-10-31 14:40     ` Dave Hansen
2018-10-31 16:42     ` Michal Hocko
2018-10-31 16:42       ` Michal Hocko
2018-11-01 11:28       ` Vovo Yang
2018-11-01 11:28         ` Vovo Yang
2018-11-01 13:09         ` Michal Hocko
2018-11-01 13:09           ` Michal Hocko
2018-11-02 12:35           ` Vovo Yang
2018-11-02 18:26             ` Michal Hocko [this message]
2018-11-02 18:26               ` Michal Hocko
2018-10-31 14:28 ` ✓ Fi.CI.IGT: success for mm, drm/i915: Mark pinned shmemfs pages as unevictable (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181102182616.GJ28039@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=dave.hansen@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterz@infradead.org \
    --cc=vovoy@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.