From: Waiman Long <email@example.com> To: Linus Torvalds <firstname.lastname@example.org>, "Huang, Ying" <email@example.com> Cc: Matthew Wilcox <firstname.lastname@example.org>, "Chen, Rong A" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, LKML <firstname.lastname@example.org>, Andi Kleen <email@example.com>, Dave Hansen <firstname.lastname@example.org>, Tim C Chen <email@example.com> Subject: Re: [LKP] [page cache] eb797a8ee0: vm-scalability.throughput -16.5% regression Date: Tue, 26 Feb 2019 15:29:42 -0500 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <CAHk-=whn+F+vFgCR0a04atTQuBmXhq0oU1Y1f0YqMiUFHj28JQ@mail.gmail.com> On 02/26/2019 12:30 PM, Linus Torvalds wrote: > On Tue, Feb 26, 2019 at 12:17 AM Huang, Ying <email@example.com> wrote: >> As for fixing. Should we care about the cache line alignment of struct >> inode? Or its size is considered more important because there may be a >> huge number of struct inode in the system? > Thanks for the great analysis. > > I suspect we _would_ like to make sure inodes are as small as > possible, since they are everywhere. Also, they are usually embedded > in other structures (ie "struct inode" is embedded into "struct > ext4_inode_info"), and unless we force alignment (and thus possibly > lots of padding), the actual alignment of 'struct inode' will vary > depending on filesystem. > > So I would suggest we *not* do cacheline alignment, because it will > result in random padding. > > But it sounds like maybe the solution is to make sure that the > different fields of the inode can and should be packed differently? > > So one thing to look at is to see what fields in 'struct inode' might > be best moved together, to minimize cache accesses. > > And in particular, if this is *only* an issue of "struct > rw_semaphore", maybe we should look at the layout of *that*. In > particular, I'm getting the feeling that we should put the "owner" > field right next to the "count" field, because the normal > non-contended path only touches those two fields. That is true. Putting the two next to each other reduces the chance of needing to touch 2 cachelines to acquire a rwsem. > Right now those two fields are pretty far from each other in 'struct > rw_semaphore', which then makes the "oops they got allocated in > different cachelines" much more likely. > > So even if 'struct inode' layout itself isn't changed, maybe just > optimizing the layout of 'struct rw_semaphore' a bit for the common > case might fix it all up. > > Waiman, I didn't check if your rewrite already possibly fixes this? My current patch doesn't move the owner field, but I will add one to do it. That change alone probably won't solve the regression we see here. The optimistic spinner is spinning on the on_cpu flag of the task structure as well as the rwsem->owner value (looking for change). The lock holder only need to touch the count/owner values once at unlock. However, if other hot data variables are in the same cacheline as rwsem->owner, we will have cacaheline bouncing problem. So we need to pad some rarely touched variables right before the rwsem in order to reduce the chance of false cacheline sharing. -Longman
next prev parent reply other threads:[~2019-02-26 20:29 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-14 9:22 kernel test robot 2018-11-14 14:17 ` Matthew Wilcox 2019-02-26 8:17 ` Huang, Ying 2019-02-26 17:30 ` Linus Torvalds 2019-02-26 20:29 ` Waiman Long [this message] 2019-02-28 1:18 ` Huang, Ying 2019-02-28 1:32 ` Linus Torvalds 2019-03-02 8:26 ` Huang, Ying 2019-02-28 2:37 ` Waiman Long 2019-02-28 3:26 ` Huang, Ying
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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [LKP] [page cache] eb797a8ee0: vm-scalability.throughput -16.5% regression' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).