From: Feng Tang <feng.tang@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>, Peter Xu <peterx@redhat.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
kernel test robot <oliver.sang@intel.com>,
John Hubbard <jhubbard@nvidia.com>,
linux-kernel@vger.kernel.org, lkp@lists.01.org,
Peter Xu <peterx@redhat.com>
Subject: Re: [PATCH] mm: relocate 'write_protect_seq' in struct mm_struct
Date: Tue, 15 Jun 2021 09:11:03 +0800 [thread overview]
Message-ID: <20210615011102.GA38780@shbuild999.sh.intel.com> (raw)
In-Reply-To: <20210614032738.GA72794@shbuild999.sh.intel.com>
On Mon, Jun 14, 2021 at 11:27:39AM +0800, Feng Tang wrote:
> >
> > It seems Ok to me, but didn't we earlier add the has_pinned which
> > would have changed the layout too? Are we chasing performance delta's
> > nobody cares about?
>
> Good point! I checked my email folder for 0day's reports, and haven't
> found a report related with Peter's commit 008cfe4418b3 ("mm: Introduce
> mm_struct.has_pinned) which adds 'has_pinned' field.
>
> Will run the same test for it and report back.
I run the same will-it-scale/mmap1 case for Peter's commit 008cfe4418b3
and its parent commit, and there is no obvious performance diff:
a1bffa48745afbb5 008cfe4418b3dbda2ff820cdd7b
---------------- ---------------------------
344353 -0.4% 342929 will-it-scale.48.threads
7173 -0.4% 7144 will-it-scale.per_thread_ops
And from the pahole info for the 2 kernels, Peter's commit adds the
'has_pinned' is put into an existing 4 bytes hole, so all other following
fields keep their alignment unchanged. Peter may do it purposely
considering the alignment. So no performance change is expected.
Pahole info for kernel before 008cfe4418b3:
struct mm_struct {
...
/* --- cacheline 1 boundary (64 bytes) --- */
long unsigned int task_size; /* 64 8 */
long unsigned int highest_vm_end; /* 72 8 */
pgd_t * pgd; /* 80 8 */
atomic_t membarrier_state; /* 88 4 */
atomic_t mm_users; /* 92 4 */
atomic_t mm_count; /* 96 4 */
/* XXX 4 bytes hole, try to pack */
atomic_long_t pgtables_bytes; /* 104 8 */
int map_count; /* 112 4 */
spinlock_t page_table_lock; /* 116 4 */
struct rw_semaphore mmap_lock; /* 120 40 */
/* --- cacheline 2 boundary (128 bytes) was 32 bytes ago --- */
pahold info with 008cfe4418b3:
struct mm_struct {
...
/* --- cacheline 1 boundary (64 bytes) --- */
long unsigned int task_size; /* 64 8 */
long unsigned int highest_vm_end; /* 72 8 */
pgd_t * pgd; /* 80 8 */
atomic_t membarrier_state; /* 88 4 */
atomic_t mm_users; /* 92 4 */
atomic_t mm_count; /* 96 4 */
atomic_t has_pinned; /* 100 4 */
atomic_long_t pgtables_bytes; /* 104 8 */
int map_count; /* 112 4 */
spinlock_t page_table_lock; /* 116 4 */
struct rw_semaphore mmap_lock; /* 120 40 */
/* --- cacheline 2 boundary (128 bytes) was 32 bytes ago --- */
Thanks,
Feng
WARNING: multiple messages have this Message-ID (diff)
From: Feng Tang <feng.tang@intel.com>
To: lkp@lists.01.org
Subject: Re: [PATCH] mm: relocate 'write_protect_seq' in struct mm_struct
Date: Tue, 15 Jun 2021 09:11:03 +0800 [thread overview]
Message-ID: <20210615011102.GA38780@shbuild999.sh.intel.com> (raw)
In-Reply-To: <20210614032738.GA72794@shbuild999.sh.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2878 bytes --]
On Mon, Jun 14, 2021 at 11:27:39AM +0800, Feng Tang wrote:
> >
> > It seems Ok to me, but didn't we earlier add the has_pinned which
> > would have changed the layout too? Are we chasing performance delta's
> > nobody cares about?
>
> Good point! I checked my email folder for 0day's reports, and haven't
> found a report related with Peter's commit 008cfe4418b3 ("mm: Introduce
> mm_struct.has_pinned) which adds 'has_pinned' field.
>
> Will run the same test for it and report back.
I run the same will-it-scale/mmap1 case for Peter's commit 008cfe4418b3
and its parent commit, and there is no obvious performance diff:
a1bffa48745afbb5 008cfe4418b3dbda2ff820cdd7b
---------------- ---------------------------
344353 -0.4% 342929 will-it-scale.48.threads
7173 -0.4% 7144 will-it-scale.per_thread_ops
And from the pahole info for the 2 kernels, Peter's commit adds the
'has_pinned' is put into an existing 4 bytes hole, so all other following
fields keep their alignment unchanged. Peter may do it purposely
considering the alignment. So no performance change is expected.
Pahole info for kernel before 008cfe4418b3:
struct mm_struct {
...
/* --- cacheline 1 boundary (64 bytes) --- */
long unsigned int task_size; /* 64 8 */
long unsigned int highest_vm_end; /* 72 8 */
pgd_t * pgd; /* 80 8 */
atomic_t membarrier_state; /* 88 4 */
atomic_t mm_users; /* 92 4 */
atomic_t mm_count; /* 96 4 */
/* XXX 4 bytes hole, try to pack */
atomic_long_t pgtables_bytes; /* 104 8 */
int map_count; /* 112 4 */
spinlock_t page_table_lock; /* 116 4 */
struct rw_semaphore mmap_lock; /* 120 40 */
/* --- cacheline 2 boundary (128 bytes) was 32 bytes ago --- */
pahold info with 008cfe4418b3:
struct mm_struct {
...
/* --- cacheline 1 boundary (64 bytes) --- */
long unsigned int task_size; /* 64 8 */
long unsigned int highest_vm_end; /* 72 8 */
pgd_t * pgd; /* 80 8 */
atomic_t membarrier_state; /* 88 4 */
atomic_t mm_users; /* 92 4 */
atomic_t mm_count; /* 96 4 */
atomic_t has_pinned; /* 100 4 */
atomic_long_t pgtables_bytes; /* 104 8 */
int map_count; /* 112 4 */
spinlock_t page_table_lock; /* 116 4 */
struct rw_semaphore mmap_lock; /* 120 40 */
/* --- cacheline 2 boundary (128 bytes) was 32 bytes ago --- */
Thanks,
Feng
next prev parent reply other threads:[~2021-06-15 3:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 1:54 [PATCH] mm: relocate 'write_protect_seq' in struct mm_struct Feng Tang
2021-06-11 1:54 ` Feng Tang
2021-06-11 17:09 ` Jason Gunthorpe
2021-06-11 17:09 ` Jason Gunthorpe
2021-06-14 3:27 ` Feng Tang
2021-06-14 3:27 ` Feng Tang
2021-06-15 1:11 ` Feng Tang [this message]
2021-06-15 1:11 ` Feng Tang
2021-06-15 18:52 ` Peter Xu
2021-06-15 18:52 ` Peter Xu
2021-06-16 1:51 ` Feng Tang
2021-06-16 1:51 ` Feng Tang
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=20210615011102.GA38780@shbuild999.sh.intel.com \
--to=feng.tang@intel.com \
--cc=akpm@linux-foundation.org \
--cc=jgg@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@lists.01.org \
--cc=oliver.sang@intel.com \
--cc=peterx@redhat.com \
--cc=torvalds@linux-foundation.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.