On Mon, Jul 20, 2015 at 04:24:00PM +0200, Joerg Roedel wrote:
> On Fri, Jul 17, 2015 at 04:32:40PM -0500, Bjorn Helgaas wrote:
> > There's no need to BUG() if we enable ATS when it's already enabled. We
> > don't need to BUG() when disabling ATS on a device that doesn't support ATS
> > or if it's already disabled. If ATS is enabled, certainly we found an ATS
> > capability in the past, so it should still be there now.
> >
> > Clean up these error paths.
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > ---
> > drivers/pci/ats.c | 7 ++-----
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> > index c35de8e..9069126 100644
> > --- a/drivers/pci/ats.c
> > +++ b/drivers/pci/ats.c
> > @@ -43,8 +43,6 @@ int pci_enable_ats(struct pci_dev *dev, int ps)
> > {
> > u16 ctrl;
> >
> > - BUG_ON(dev->ats_cap && dev->ats_enabled);
> > -
> > if (!dev->ats_cap)
> > return -EINVAL;
>
> Hmm, what happens if pci_enable_ats is called twice for the same
> device? Can we change the STU without disabling ATS first, for example?
>
> The function should probably return if it finds ATS already enabled (or
> at least WARN when it is enabled and ps != last ps value).
It would probably be nicest if:
- enabling PF fails if ATS is already enabled
- enabling VF fails if (PF ATS is disabled || PS != PF PS)
- disabling PF fails if ATS is enabled on any VF
I added the first (which we basically had before in the BUG_ON()), and I
already had the second.
The third is where it gets messy because we have either add a refcount and
associated locking, or we have to iterate through all the VFs. I did
consider both of those but thought it was a lot of work for the way it's
currently used: it looks like ATS is enabled for every device when it is
enumerated, with a fixed PS, and never changed.
But I guess a refcount with atomic_inc() probably wouldn't be too hard.
Let me poke at that.
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
