Re: [PATCH v4 1/2] IB/qib: add acounting for MTRR

Luis R. Rodriguez
2015-04-22T15:28:21Z

On Wed, Apr 22, 2015 at 09:44:52AM -0400, Doug Ledford wrote:
> On Tue, 2015-04-21 at 14:50 -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > There is no good reason not to, we eventually delete it as well.
> >
> > Cc: Toshi Kani <toshi.kani@hp.com>
> > Cc: Suresh Siddha <sbsiddha@gmail.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Mike Marciniszyn <infinipath@intel.com>
> > Cc: Roland Dreier <roland@kernel.org>
> > Cc: Sean Hefty <sean.hefty@intel.com>
> > Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
> > Cc: linux-rdma@vger.kernel.org
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > ---
> > drivers/infiniband/hw/qib/qib_wc_x86_64.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/hw/qib/qib_wc_x86_64.c b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
> > index 81b225f..fe0850a 100644
> > --- a/drivers/infiniband/hw/qib/qib_wc_x86_64.c
> > +++ b/drivers/infiniband/hw/qib/qib_wc_x86_64.c
> > @@ -118,7 +118,7 @@ int qib_enable_wc(struct qib_devdata *dd)
> > if (!ret) {
> > int cookie;
> >
> > - cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
> > + cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
> > if (cookie < 0) {
> > {
> > qib_devinfo(dd->pcidev,
>
> Skip this patch please. You remove this line entirely in your next
> patch, so this becomes a single kernel out of all possible bisectable
> kernels with this accounting enabled, and then the very next kernel does
> away with it.

No, the next patch uses accounting enabled as well, it also makes somse
other changes. This change is done in order to add accounting to match
the grammar used by arch_phys_wc_add() so it is in fact an atomic
comittiable and highly recommmended bisectable commit to be present.

> It makes no sense to have a single outlying bisectable
> kernel like that.

This is an atomic difference worth keeping record of.

Luis