Re: [PATCH 1/2] PCI/DPC: Add 'nodpc' parameter

["Derrick, Jonathan" <jonathan.derrick@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3851 bytes --]

Hi Bjorn,

Thanks for your pov.

I'm going a lot of different ways on this one, but it seems like the
most reasonable option right now is to drop it until someone actually
presents a non-debugging need to disable DPC from sysfs or globally.

Best regards,
Jon

On Fri, 2018-08-17 at 09:25 -0500, Bjorn Helgaas wrote:
> On Thu, Aug 16, 2018 at 08:50:42PM +0000, Derrick, Jonathan wrote:
> > On Thu, 2018-08-16 at 15:31 -0500, Bjorn Helgaas wrote:
> > > On Thu, Aug 16, 2018 at 03:50:47PM +0000, Derrick, Jonathan
> > > wrote:
> > > > On Thu, 2018-08-16 at 08:49 -0700, Matthew Wilcox wrote:
> > > > > On Wed, Aug 15, 2018 at 03:26:39PM -0600, Jon Derrick wrote:
> > > > > > Some users may want to disable downstream port containment
> > > > > > (DPC),
> > > > > > so
> > > > > > give them this option
> > > > > 
> > > > > Is it possible they might only want to disable DPC on a
> > > > > subset of
> > > > > the
> > > > > hierarchy rather than globally?
> > > > 
> > > > Absolutely. I was hoping Logan's pci dev_str would land because
> > > > I
> > > > have
> > > > a few others I want to convert to that api for granular tuning
> > > 
> > > What's the use case here?  I acknowledge there are cases where we
> > > need
> > > them, but I'm not a fan of kernel parameters in general because
> > > they're a real hassle for users.
> > > 
> > > Is there something wrong with DPC?  Is there some way we can make
> > > it
> > > smarter so it does the right thing automatically?
> > 
> > I've encountered some BIOS or switches (not sure who) who've
> > appeared
> > to have enabled DPC by default, prior to kernel setup. Some users
> > may
> > just not want this, but it was primarily intended for debugging
> > when I
> > conceived it.
> > 
> > There's also the ongoing thread in linux-pci about err handling in
> > PPC
> > EEH, who may also desire to disable DPC until those issues are
> > resolved.
> 
> I haven't caught up on that thread yet.  If DPC is incompatible with
> PPC EEH, there's always the possibility of a switch in the code to
> disable DPC automatically on PPC.
> 
> > It can also be disabled with setpci, but is that any less of a
> > hassle?
> > Genuine question to understand your point of view.
> 
> Keith already answered here; setpci is primarily for debugging and
> shouldn't be recommended as normal practice.
> 
> > > I'm more OK with a blanket "nodpc" switch intended for debugging.
> > > If we add the complexity of subsets of the hierarchy it starts
> > > sounding like an administrative thing that makes me more
> > > hesitant. 
> > 
> > ...
> > To again understand your point of view, is there anything wrong
> > with
> > administrative things in kernel boot parameters? There will be
> > cases
> > where we may want to deviate from default or global pci=*
> > parameters
> > for certain hierarchies and they can't necessarily be set after the
> > fact (ex, hpmemsize).
> 
> "There will be cases" sounds like we're doing something that *might*
> be useful in the future.  It's better if we wait until we actually
> discover a need for something.
> 
> There's also a tendency among users to trip over a problem, discover
> a
> boot parameter like "pci=nomsi", and conclude that the problem is
> "fixed".  In fact, we want the bug report so we can fix the kernel so
> no boot parameter is needed.
> 
> I agree there are things that can't be set after boot.  Is this one
> of
> them?  This seems like something that could be controlled at run-
> time.
> But even there, I will ask what the requirement for it is, because we
> don't want to clutter sysfs with things only needed for debugging.
> 
> Boot parameters are a hassle because it's hard to build a user
> interface on top of them, and they require a reboot to take effect.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3278 bytes --]