Re: [net-next 10/10] net/mlx5e: Add support for PCI relaxed ordering

From: Bjorn Helgaas <helgaas@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Aya Levin <ayal@mellanox.com>,
	Saeed Mahameed <saeedm@mellanox.com>,
	"mkubecek@suse.cz" <mkubecek@suse.cz>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Tariq Toukan <tariqt@mellanox.com>,
	linux-pci@vger.kernel.org,
	Alexander Duyck <alexander.h.duyck@linux.intel.com>
Subject: Re: [net-next 10/10] net/mlx5e: Add support for PCI relaxed ordering
Date: Fri, 26 Jun 2020 15:12:54 -0500	[thread overview]
Message-ID: <20200626201254.GA2932090@bjorn-Precision-5520> (raw)
In-Reply-To: <20200624102258.4410008d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed, Jun 24, 2020 at 10:22:58AM -0700, Jakub Kicinski wrote:
> On Wed, 24 Jun 2020 10:34:40 +0300 Aya Levin wrote:
> > >> I think Michal will rightly complain that this does not belong in
> > >> private flags any more. As (/if?) ARM deployments take a foothold
> > >> in DC this will become a common setting for most NICs.  
> > > 
> > > Initially we used pcie_relaxed_ordering_enabled() to
> > >   programmatically enable this on/off on boot but this seems to
> > > introduce some degradation on some Intel CPUs since the Intel Faulty
> > > CPUs list is not up to date. Aya is discussing this with Bjorn.  
> > Adding Bjorn Helgaas
> 
> I see. Simply using pcie_relaxed_ordering_enabled() and blacklisting
> bad CPUs seems far nicer from operational perspective. Perhaps Bjorn
> will chime in. Pushing the validation out to the user is not a great
> solution IMHO.

I'm totally lost, but maybe it doesn't matter because it looks like
David has pulled this series already.

There probably *should* be a PCI core interface to enable RO, but
there isn't one today.

pcie_relaxed_ordering_enabled() doesn't *enable* anything.  All it
does is tell you whether RO is already enabled.

This patch ([net-next 10/10] net/mlx5e: Add support for PCI relaxed
ordering) apparently adds a knob to control RO, but I can't connect
the dots.  It doesn't touch PCI_EXP_DEVCTL_RELAX_EN, and that symbol
doesn't occur anywhere in drivers/net except tg3, myri10ge, and niu.

And this whole series doesn't contain PCI_EXP_DEVCTL_RELAX_EN or
pcie_relaxed_ordering_enabled().

I do have a couple emails from Aya, but they didn't include a patch
and I haven't quite figured out what the question was.

> > > So until we figure this out, will keep this off by default.
> > > 
> > > for the private flags we want to keep them for performance analysis as
> > > we do with all other mlx5 special performance features and flags.