3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Sander Eikelenboom]

Hi.

In 3.19 the device name associates with IRQ's for ahci controllers operating
with a single IRQ changed from "ahci?" to "<BDF>", was this intentional ?

It's probably commit 18dcf433f3ded61eb140a55e7048ec2fef79e723 (or another one
in that series). 

--
Sander


/proc/interrupts of an ahci controller with a single irq:
 52:      13529          0          0          0  xen-pirq-msi       0000:00:1f.2

/proc/interrupts of an ahci controller with multiple irq's:
 114:     412535          0          0          0          0          0  xen-pirq-msi       ahci0
 115:          0          0          0          0          0          0  xen-pirq-msi       ahci1
 116:      16717          0          0          0          0          0  xen-pirq-msi       ahci2
 117:          0          0          0          0          0          0  xen-pirq-msi       ahci3
 118:          0          0          0          0          0          0  xen-pirq-msi       ahci4
 119:          0          0          0          0          0          0  xen-pirq-msi       ahci5

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Stefan Bader]

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

On 09.02.2015 20:54, Sander Eikelenboom wrote:
> Hi.
> 
> In 3.19 the device name associates with IRQ's for ahci controllers operating
> with a single IRQ changed from "ahci?" to "<BDF>", was this intentional ?
> 
> It's probably commit 18dcf433f3ded61eb140a55e7048ec2fef79e723 (or another one
> in that series). 

Oh, looking at that commit that might make sense. In
ahci_host_activate_single_irq it now uses dev_driver_name instead of dev_name as
it did before (from ata_host_activate).  The description sounds like before the
driver is registered this will return the bus. And registering an interrupt
would likely be before the driver is fully registered... But you probably saw
that, too. Whether that really was intentional is still the question. :)

-Stefan

>
> --
> Sander
> 
> 
> /proc/interrupts of an ahci controller with a single irq:
>  52:      13529          0          0          0  xen-pirq-msi       0000:00:1f.2
> 
> /proc/interrupts of an ahci controller with multiple irq's:
>  114:     412535          0          0          0          0          0  xen-pirq-msi       ahci0
>  115:          0          0          0          0          0          0  xen-pirq-msi       ahci1
>  116:      16717          0          0          0          0          0  xen-pirq-msi       ahci2
>  117:          0          0          0          0          0          0  xen-pirq-msi       ahci3
>  118:          0          0          0          0          0          0  xen-pirq-msi       ahci4
>  119:          0          0          0          0          0          0  xen-pirq-msi       ahci5
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Tejun Heo]

Hello,

On Tue, Feb 10, 2015 at 08:57:46AM +0100, Stefan Bader wrote:
> On 09.02.2015 20:54, Sander Eikelenboom wrote:
> > Hi.
> > 
> > In 3.19 the device name associates with IRQ's for ahci controllers operating
> > with a single IRQ changed from "ahci?" to "<BDF>", was this intentional ?
> > 
> > It's probably commit 18dcf433f3ded61eb140a55e7048ec2fef79e723 (or another one
> > in that series). 
> 
> Oh, looking at that commit that might make sense. In
> ahci_host_activate_single_irq it now uses dev_driver_name instead of dev_name as
> it did before (from ata_host_activate).  The description sounds like before the
> driver is registered this will return the bus. And registering an interrupt
> would likely be before the driver is fully registered... But you probably saw
> that, too. Whether that really was intentional is still the question. :)

Urgh... Alexander, can you please send a patch to fix up the names?

Thanks.

-- 
tejun

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Alexander Gordeev]

On Tue, Feb 10, 2015 at 09:51:20AM -0500, Tejun Heo wrote:
> Hello,
> 
> On Tue, Feb 10, 2015 at 08:57:46AM +0100, Stefan Bader wrote:
> > On 09.02.2015 20:54, Sander Eikelenboom wrote:
> > > Hi.
> > > 
> > > In 3.19 the device name associates with IRQ's for ahci controllers operating
> > > with a single IRQ changed from "ahci?" to "<BDF>", was this intentional ?
> > > 
> > > It's probably commit 18dcf433f3ded61eb140a55e7048ec2fef79e723 (or another one
> > > in that series). 
> > 
> > Oh, looking at that commit that might make sense. In
> > ahci_host_activate_single_irq it now uses dev_driver_name instead of dev_name as
> > it did before (from ata_host_activate).  The description sounds like before the
> > driver is registered this will return the bus. And registering an interrupt
> > would likely be before the driver is fully registered... But you probably saw
> > that, too. Whether that really was intentional is still the question. :)
> 
> Urgh... Alexander, can you please send a patch to fix up the names?

Will try to send over the weekend.

> Thanks.
> 
> -- 
> tejun

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Alexander Gordeev]

On Tue, Feb 10, 2015 at 09:51:20AM -0500, Tejun Heo wrote:
> Hello,
> 
> On Tue, Feb 10, 2015 at 08:57:46AM +0100, Stefan Bader wrote:
> > On 09.02.2015 20:54, Sander Eikelenboom wrote:
> > > Hi.
> > > 
> > > In 3.19 the device name associates with IRQ's for ahci controllers operating
> > > with a single IRQ changed from "ahci?" to "<BDF>", was this intentional ?
> > > 
> > > It's probably commit 18dcf433f3ded61eb140a55e7048ec2fef79e723 (or another one
> > > in that series). 
> > 
> > Oh, looking at that commit that might make sense. In
> > ahci_host_activate_single_irq it now uses dev_driver_name instead of dev_name as
> > it did before (from ata_host_activate).  The description sounds like before the
> > driver is registered this will return the bus. And registering an interrupt
> > would likely be before the driver is fully registered... But you probably saw
> > that, too. Whether that really was intentional is still the question. :)
> 
> Urgh... Alexander, can you please send a patch to fix up the names?

The problem dates back to commit 4f37b50 ("libata: Use dev_name() for
request_irq() to distinguish devices"). If the commit is considered valid
then <BDF> still might be legitimate as well. Not sure what is the best
approach here.

> Thanks.
> 
> -- 
> tejun

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Tejun Heo]

Hello,

On Mon, Feb 16, 2015 at 05:28:42PM +0000, Alexander Gordeev wrote:
> The problem dates back to commit 4f37b50 ("libata: Use dev_name() for
> request_irq() to distinguish devices"). If the commit is considered valid
> then <BDF> still might be legitimate as well. Not sure what is the best
> approach here.

What does <BDF> indicate?  Does it successfully identify the device in
question?

Thanks.

-- 
tejun

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Alexander Gordeev]

On Mon, Mar 02, 2015 at 12:21:08PM -0500, Tejun Heo wrote:
> Hello,
> 
> On Mon, Feb 16, 2015 at 05:28:42PM +0000, Alexander Gordeev wrote:
> > The problem dates back to commit 4f37b50 ("libata: Use dev_name() for
> > request_irq() to distinguish devices"). If the commit is considered valid
> > then <BDF> still might be legitimate as well. Not sure what is the best
> > approach here.
> 
> What does <BDF> indicate?  Does it successfully identify the device in
> question?

I suppose, PCI bus-device-function.
Yes, it does.

> Thanks.
> 
> -- 
> tejun

-- 
Regards,
Alexander Gordeev
agordeev@redhat.com

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Tejun Heo]

On Mon, Mar 02, 2015 at 06:58:10PM +0000, Alexander Gordeev wrote:
> On Mon, Mar 02, 2015 at 12:21:08PM -0500, Tejun Heo wrote:
> > Hello,
> > 
> > On Mon, Feb 16, 2015 at 05:28:42PM +0000, Alexander Gordeev wrote:
> > > The problem dates back to commit 4f37b50 ("libata: Use dev_name() for
> > > request_irq() to distinguish devices"). If the commit is considered valid
> > > then <BDF> still might be legitimate as well. Not sure what is the best
> > > approach here.
> > 
> > What does <BDF> indicate?  Does it successfully identify the device in
> > question?
> 
> I suppose, PCI bus-device-function.
> Yes, it does.

Hmmm... it does stand out like a sore thumb and we probably should use
a more descriptive identifier including the driver name.  4f37b50
("libata: Use dev_name() was fine for the specific platform device as
the device identifier included ide but not generally.  :(

How about driver name followed by device ID?

Thanks.

-- 
tejun

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Sander Eikelenboom]

Monday, March 2, 2015, 7:58:10 PM, you wrote:

> On Mon, Mar 02, 2015 at 12:21:08PM -0500, Tejun Heo wrote:
>> Hello,
>> 
>> On Mon, Feb 16, 2015 at 05:28:42PM +0000, Alexander Gordeev wrote:
>> > The problem dates back to commit 4f37b50 ("libata: Use dev_name() for
>> > request_irq() to distinguish devices"). If the commit is considered valid
>> > then <BDF> still might be legitimate as well. Not sure what is the best
>> > approach here.
>> 
>> What does <BDF> indicate?  Does it successfully identify the device in
>> question?

> I suppose, PCI bus-device-function.
> Yes, it does.

But at least it should be the same for single irq and not single irq,
which at present doesn't seem to be the case.

Apart from that you could also combine the devicetype with the BDF,
like xen-pciback does that looks like this in /proc/interrupts:
  22:     226798          0          0          0          0          0  xen-pirq-ioapic-level  xen-pciback[0000:03:06.0]

--
Sander

>> Thanks.
>> 
>> -- 
>> tejun

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Tejun Heo]

On Mon, Mar 02, 2015 at 08:11:00PM +0100, Sander Eikelenboom wrote:
> But at least it should be the same for single irq and not single irq,
> which at present doesn't seem to be the case.

Yeah, I agree the current situation is broken.

Thanks.

-- 
tejun

Re: 3.19: device name associates with IRQ's for ahci controllers operating with a single IRQ changed from "ahci?" to "<BDF>"

[Tejun Heo]

On Mon, Mar 02, 2015 at 02:10:49PM -0500, Tejun Heo wrote:
> Hmmm... it does stand out like a sore thumb and we probably should use
> a more descriptive identifier including the driver name.  4f37b50
> ("libata: Use dev_name() was fine for the specific platform device as
> the device identifier included ide but not generally.  :(
> 
> How about driver name followed by device ID?

Or we can move print_id allocation to host allocation time and do

 ata%d-%d:DRIVER_NAME

-- 
tejun