All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Juergen Gross <jgross@suse.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	hpa@zytor.com, x86@kernel.org, mingo@redhat.com,
	stefan.bader@canonical.com, linux-kernel@vger.kernel.org,
	xen-devel@lists.xensource.com, konrad.wilk@oracle.com,
	ville.syrjala@linux.intel.com, david.vrabel@citrix.com,
	jbeulich@suse.com, toshi.kani@hp.com
Subject: Re: [PATCH V4 1/3] x86: Make page cache mode a real type
Date: Wed, 29 Oct 2014 11:36:05 +0100	[thread overview]
Message-ID: <20141029103605.GA11648@gmail.com> (raw)
In-Reply-To: <5450C2F3.40400@suse.com>


* Juergen Gross <jgross@suse.com> wrote:

> On 10/29/2014 11:28 AM, Ingo Molnar wrote:
> >
> >* Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> >>On Mon, 27 Oct 2014, Juergen Gross wrote:
> >>>At the moment there are a lot of places that handle setting or getting
> >>>the page cache mode by treating the pgprot bits equal to the cache mode.
> >>>This is only true because there are a lot of assumptions about the setup
> >>>of the PAT MSR. Otherwise the cache type needs to get translated into
> >>>pgprot bits and vice versa.
> >>>
> >>>This patch tries to prepare for that by introducing a separate type
> >>>for the cache mode and adding functions to translate between those and
> >>>pgprot values.
> >>>
> >>>To avoid too much performance penalty the translation between cache mode
> >>>and pgprot values is done via tables which contain the relevant
> >>>information.  Write-back cache mode is hard-wired to be 0, all other
> >>>modes are configurable via those tables. For large pages there are
> >>>translation functions as the PAT bit is located at different positions
> >>>in the ptes of 4k and large pages.
> >>
> >>I'm fine with the approach itself. Though I wish you had split this
> >>patch in several sanely to review pieces.
> >>
> >>   - Introduce enums, helper functions etc. which basically reflect
> >>     the state of today
> >>
> >>   - Change the usage sites to use the enums and helpers
> >>
> >>   - Convert the enum/helper implementation to the new scheme
> >>
> >>   - Split out the printk change
> >>
> >>   ...
> >
> >That absolutely has to be done, should any of this introduce
> >regressions. The diffstat:
> >
> >  18 files changed, 344 insertions(+), 212 deletions(-)
> >
> >is _way_ too large.
> 
> Okay, will do.

Thanks!

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Juergen Gross <jgross@suse.com>
Cc: xen-devel@lists.xensource.com, toshi.kani@hp.com, x86@kernel.org,
	linux-kernel@vger.kernel.org, stefan.bader@canonical.com,
	mingo@redhat.com, david.vrabel@citrix.com, jbeulich@suse.com,
	hpa@zytor.com, Thomas Gleixner <tglx@linutronix.de>,
	ville.syrjala@linux.intel.com
Subject: Re: [PATCH V4 1/3] x86: Make page cache mode a real type
Date: Wed, 29 Oct 2014 11:36:05 +0100	[thread overview]
Message-ID: <20141029103605.GA11648@gmail.com> (raw)
In-Reply-To: <5450C2F3.40400@suse.com>


* Juergen Gross <jgross@suse.com> wrote:

> On 10/29/2014 11:28 AM, Ingo Molnar wrote:
> >
> >* Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> >>On Mon, 27 Oct 2014, Juergen Gross wrote:
> >>>At the moment there are a lot of places that handle setting or getting
> >>>the page cache mode by treating the pgprot bits equal to the cache mode.
> >>>This is only true because there are a lot of assumptions about the setup
> >>>of the PAT MSR. Otherwise the cache type needs to get translated into
> >>>pgprot bits and vice versa.
> >>>
> >>>This patch tries to prepare for that by introducing a separate type
> >>>for the cache mode and adding functions to translate between those and
> >>>pgprot values.
> >>>
> >>>To avoid too much performance penalty the translation between cache mode
> >>>and pgprot values is done via tables which contain the relevant
> >>>information.  Write-back cache mode is hard-wired to be 0, all other
> >>>modes are configurable via those tables. For large pages there are
> >>>translation functions as the PAT bit is located at different positions
> >>>in the ptes of 4k and large pages.
> >>
> >>I'm fine with the approach itself. Though I wish you had split this
> >>patch in several sanely to review pieces.
> >>
> >>   - Introduce enums, helper functions etc. which basically reflect
> >>     the state of today
> >>
> >>   - Change the usage sites to use the enums and helpers
> >>
> >>   - Convert the enum/helper implementation to the new scheme
> >>
> >>   - Split out the printk change
> >>
> >>   ...
> >
> >That absolutely has to be done, should any of this introduce
> >regressions. The diffstat:
> >
> >  18 files changed, 344 insertions(+), 212 deletions(-)
> >
> >is _way_ too large.
> 
> Okay, will do.

Thanks!

	Ingo

  reply	other threads:[~2014-10-29 10:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-27  6:48 [PATCH V4 0/3] x86: Full support of PAT Juergen Gross
2014-10-27  6:48 ` [PATCH V4 1/3] x86: Make page cache mode a real type Juergen Gross
2014-10-28 17:05   ` Thomas Gleixner
2014-10-29 10:28     ` Ingo Molnar
2014-10-29 10:35       ` Juergen Gross
2014-10-29 10:36         ` Ingo Molnar [this message]
2014-10-29 10:36           ` Ingo Molnar
2014-10-27  6:48 ` [PATCH V4 2/3] x86: Enable PAT to use cache mode translation tables Juergen Gross
2014-10-29 10:56   ` Ingo Molnar
2014-10-27  6:48 ` [PATCH V4 3/3] Support Xen pv-domains using PAT Juergen Gross

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=20141029103605.GA11648@gmail.com \
    --to=mingo@kernel.org \
    --cc=david.vrabel@citrix.com \
    --cc=hpa@zytor.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=stefan.bader@canonical.com \
    --cc=tglx@linutronix.de \
    --cc=toshi.kani@hp.com \
    --cc=ville.syrjala@linux.intel.com \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /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.