The pm80xx driver hangs in 3.10 with the Adaptec 71605H HBA
Hi!
I am trying to get my Adaptec 71605H HBA to work with kernel 3.10. Unfortunately it hangs while waiting for a completion.
After registering all the interrupts the pm8001_pci_probe() function calls pm8001_init_sas_add(). That function calls PM8001_CHIP_DISP->get_nvmd_req() and waits for a completion that never happens.
I tried to use the drivers available from the Adaptec website, and after hacking them so they compiled again with 3.10 I just copied those sources over to the kernel, recompiled and tested those. And lo and behold, that actually worked! So the Adaptec drivers do something that the current kernel drivers don't.
The patch I applied to the 3.10 kernel to effectively update the current drivers with those from Adaptec is here:
http://hverkuil.home.xs4all.nl/0002-pm80xx-copy-over-adaptec-drivers.patch
The original drivers from Adaptec are here:
http://www.adaptec.com/en-us/downloads/rh/rhel_6/productid=asa-71605h&dn=adaptec+71605h.html
I tried to find relevant differences that might explain why the adaptec version works while the current 3.10 doesn't, but nothing jumped out to me.
It's no problem to test any patches you have, or to do some debugging if someone can point me in the right direction.
Regards,
Hans
A message that is not in the archive (sent off-list or not yet received): CAD+HZHUc7wULDUpKq24rmcVC9MCcgaoO4i1LxGcW-q-y-c3tcQ@mail.gmail.com
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): [email protected]_nt.nt.pmc-sierra.bc.ca
Hi Anand,
Quoted 8 lines · Anand Kumar Santhanam
Hans,
I reviewed the code changes and I did not see major differences except for the fact that in adaptec driver we have 64 interrupt handlers to handle 64 MSI-X.
This was optimized in open src driver to use only 1 interrupt handler. Can you pls make this change to the open src driver (i.e have multiple interrupt handlers for multiple MSI-X) and check?
I've looked at this more closely, and I wonder whether there isn't a race condition here. When an interrupt arrives you put the interrupt vector in pm8001_ha->int_vector, then schedule the tasklet. But what if two interrupts with different vectors arrive in quick succession before the tasklet got a chance to run? In that case the tasklet will only see the second vector, not the first. Rather scary.
I have not actually seen any issues with this, but by definition race conditions are hard to reproduce and I haven't done any serious testing with this card. For now I will run with the quick and dirty msi.diff (http://hverkuil.home.xs4all.nl/msi.diff).
I see two solutions: either use the 64 interrupt handlers as done in the adaptec driver, or you can change int_vector into a u64 and use it as a bitmask to record all interrupt vectors that have arrived.
BTW, another difference between the linux kernel driver and the adaptec version are several of the defines in pm8001_defs.h: e.g. MPI_QUEUE is 256 in the adaptec driver, while it is 1024 in the kernel driver. There are other differences as well.
Are all the changes in the kernel correct? I would like to have a confirmation of that before I am going to trust my data to this driver.
It clearly hasn't been tested with actual hardware :-(
Regards,
Hans
Hi Hans,
Quoted 18 lines · Hans Verkuil
Hi Anand,
Quoted 8 lines · Anand Kumar Santhanam
Hans,
I reviewed the code changes and I did not see major differences except for the fact that in adaptec driver we have 64 interrupt handlers to handle 64 MSI-X.
This was optimized in open src driver to use only 1 interrupt handler. Can you pls make this change to the open src driver (i.e have multiple interrupt handlers for multiple MSI-X) and check?I've looked at this more closely, and I wonder whether there isn't a race condition here. When an interrupt arrives you put the interrupt vector in
pm8001_ha->int_vector, then schedule the tasklet. But what if two interrupts with different vectors arrive in quick succession before the tasklet got a chance to run? In that case the tasklet will only see the second vector, not the first. Rather scary.
Quoted 7 lines
I have not actually seen any issues with this, but by definition race conditions are hard to reproduce and I haven't done any serious testing with this card. For now I will run with the quick and dirty msi.diff (http://hverkuil.home.xs4all.nl/msi.diff).
I see two solutions: either use the 64 interrupt handlers as done in the adaptec driver, or you can change
int_vectorinto a u64 and use it as a bitmask to record all interrupt vectors that have arrived.
Thanks for looking into this, I think second one is what we want, set the bitmask when interrupt arrived and clear it when it's processed.
Quoted 5 lines
BTW, another difference between the linux kernel driver and the adaptec version are several of the defines in
pm8001_defs.h: e.g.MPI_QUEUEis 256 in the adaptec driver, while it is 1024 in the kernel driver. There are other differences as well.
Different value may reflect different performance character, but both should works, there is no one for all setting.
Quoted 5 lines
Are all the changes in the kernel correct? I would like to have a confirmation of that before I am going to trust my data to this driver.
It clearly hasn't been tested with actual hardware :-(
:_(
Thanks,
Jack
Quoted 4 lines
Regards,
Hans
Hi Hans,
Pls find responses inline.
Regards
Anand
-----Original Message-----
From: Jack Wang [mailto:[email protected]]
Sent: Monday, July 15, 2013 2:24 PM
To: Hans Verkuil
Cc: Anand Kumar Santhanam; lindar_liu; [email protected]; [email protected]
Subject: Re: The pm80xx driver hangs in 3.10 with the Adaptec 71605H HBA
Hi Hans,
Quoted 9 lines · Hans Verkuil
Hi Anand,
Quoted 6 lines · Anand Kumar Santhanam
Hans,
I reviewed the code changes and I did not see major differences except for the fact that in adaptec driver we have 64 interrupt handlers to handle 64 MSI-X.
This was optimized in open src driver to use only 1 interrupt
handler.
Quoted 8 lines
Quoted 2 lines
Can you pls make this change to the open src driver (i.e have multiple interrupt handlers for multiple MSI-X) and check?
I've looked at this more closely, and I wonder whether there isn't a race condition here. When an interrupt arrives you put the interrupt vector in
pm8001_ha->int_vector, then schedule the tasklet. But what if two interrupts with different vectors arrive in quick succession before the tasklet got a chance to run? In that case the tasklet will
only see the second vector, not the first. Rather scary.
Quoted 4 lines
I have not actually seen any issues with this, but by definition race conditions are hard to reproduce and I haven't done any serious testing with this card. For now I will run with the quick and dirty
msi.diff (http://hverkuil.home.xs4all.nl/msi.diff).
Quoted 5 lines
I see two solutions: either use the 64 interrupt handlers as done in the adaptec driver, or you can change
int_vectorinto a u64 and use itas a bitmask to record all interrupt vectors that have arrived.
Thanks for looking into this, I think second one is what we want, set the bitmask when interrupt arrived and clear it when it's processed.
Anand>> Yes. We will go for the second solution. The multiple interrupt/tasklet handlers for MSI-X got rejected by the community and hence
We went for the existing approach. I checked with a single controller and I did not observe any issues. Also pls note that the open source driver
Supports only one MSI-X for now and this problem will not occur.
Quoted 4 lines
BTW, another difference between the linux kernel driver and the adaptec version are several of the defines in
pm8001_defs.h: e.g.MPI_QUEUEis 256 in the adaptec driver, while it is 1024 in the kernel
driver. There are other differences as well.
Quoted 1 line
Different value may reflect different performance character, but both should works, there is no one for all setting.
Anand >> I agree with Jack. I will check on the #defines and get back.
Quoted 2 lines
Are all the changes in the kernel correct? I would like to have a confirmation of that before I am going to trust my data to this
driver.
Quoted 3 lines
It clearly hasn't been tested with actual hardware :-(
:_(
Quoted 1 line
Quoted 1 line
My sincere apologies. I tested the same with single controller and it
worked fine. However I messed up when submitting
The patches. This was my first open source submission and request you to bear the inconvenience.
Thanks,
Jack
Quoted 4 lines
Regards,
Hans
Hi Anand,
Quoted 72 lines · Anand Kumar Santhanam
Hi Hans,
Pls find responses inline.
Regards
Anand-----Original Message-----
From: Jack Wang [mailto:[email protected]]
Sent: Monday, July 15, 2013 2:24 PM
To: Hans Verkuil
Cc: Anand Kumar Santhanam;lindar_liu; [email protected]; [email protected]
Subject: Re: The pm80xx driver hangs in 3.10 with the Adaptec 71605H HBAHi Hans,
Quoted 9 lines · Hans Verkuil
Hi Anand,
Quoted 6 lines · Anand Kumar Santhanam
Hans,
I reviewed the code changes and I did not see major differences except for the fact that in adaptec driver we have 64 interrupt handlers to handle 64 MSI-X.
This was optimized in open src driver to use only 1 interrupthandler.
Quoted 8 lines
Quoted 2 lines
Can you pls make this change to the open src driver (i.e have multiple interrupt handlers for multiple MSI-X) and check?
I've looked at this more closely, and I wonder whether there isn't a race condition here. When an interrupt arrives you put the interrupt vector in
pm8001_ha->int_vector, then schedule the tasklet. But what if two interrupts with different vectors arrive in quick succession before the tasklet got a chance to run? In that case the tasklet willonly see the second vector, not the first. Rather scary.
Quoted 4 lines
I have not actually seen any issues with this, but by definition race conditions are hard to reproduce and I haven't done any serious testing with this card. For now I will run with the quick and dirty
msi.diff (http://hverkuil.home.xs4all.nl/msi.diff).
Quoted 5 lines
I see two solutions: either use the 64 interrupt handlers as done in the adaptec driver, or you can change
int_vectorinto a u64 and use itas a bitmask to record all interrupt vectors that have arrived.
Thanks for looking into this, I think second one is what we want, set the bitmask when interrupt arrived and clear it when it's processed.
Anand>> Yes. We will go for the second solution. The multiple interrupt/tasklet handlers for MSI-X got rejected by the community and hence
We went for the existing approach. I checked with a single controller and I did not observe any issues. Also pls note that the open source driver
Supports only one MSI-X for now and this problem will not occur.Quoted 4 lines
BTW, another difference between the linux kernel driver and the adaptec version are several of the defines in
pm8001_defs.h: e.g.MPI_QUEUEis 256 in the adaptec driver, while it is 1024 in the kerneldriver. There are other differences as well.
Quoted 1 line
Different value may reflect different performance character, but both should works, there is no one for all setting.
Anand >> I agree with Jack. I will check on the #defines and get back.
Quoted 2 lines
Are all the changes in the kernel correct? I would like to have a confirmation of that before I am going to trust my data to this
driver.
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
It clearly hasn't been tested with actual hardware :-(
:_(
Quoted 1 line
Quoted 1 line
My sincere apologies. I tested the same with single controller and it
worked fine. However I messed up when submitting
The patches. This was my first open source submission and request you to bear the inconvenience.
Never mind, people make mistake, quickly fix that should be fine. I suggest keep minimal difference between your in house driver and in tree driver, it will make us easy to maintain the driver and make it easy to use for user.
Best Regards,
Jack
Hi Anand!
Quoted 46 lines · Anand Kumar Santhanam
Hi Hans,
Pls find responses inline.
Regards
Anand-----Original Message-----
From: Jack Wang [mailto:[email protected]]
Sent: Monday, July 15, 2013 2:24 PM
To: Hans Verkuil
Cc: Anand Kumar Santhanam;lindar_liu; [email protected]; [email protected]
Subject: Re: The pm80xx driver hangs in 3.10 with the Adaptec 71605H HBAHi Hans,
Quoted 9 lines · Hans Verkuil
Hi Anand,
Quoted 6 lines · Anand Kumar Santhanam
Hans,
I reviewed the code changes and I did not see major differences except for the fact that in adaptec driver we have 64 interrupt handlers to handle 64 MSI-X.
This was optimized in open src driver to use only 1 interrupthandler.
Quoted 8 lines
Quoted 2 lines
Can you pls make this change to the open src driver (i.e have multiple interrupt handlers for multiple MSI-X) and check?
I've looked at this more closely, and I wonder whether there isn't a race condition here. When an interrupt arrives you put the interrupt vector in
pm8001_ha->int_vector, then schedule the tasklet. But what if two interrupts with different vectors arrive in quick succession before the tasklet got a chance to run? In that case the tasklet willonly see the second vector, not the first. Rather scary.
Quoted 4 lines
I have not actually seen any issues with this, but by definition race conditions are hard to reproduce and I haven't done any serious testing with this card. For now I will run with the quick and dirty
msi.diff (http://hverkuil.home.xs4all.nl/msi.diff).
Quoted 5 lines
I see two solutions: either use the 64 interrupt handlers as done in the adaptec driver, or you can change
int_vectorinto a u64 and use itas a bitmask to record all interrupt vectors that have arrived.
Quoted 8 lines
Thanks for looking into this, I think second one is what we want, set the bitmask when interrupt arrived and clear it when it's processed.
Anand>> Yes. We will go for the second solution. The multiple interrupt/tasklet handlers for MSI-X got rejected by the community and hence
We went for the existing approach. I checked with a single controller and I did not observe any issues.
Well, you typically won't see any issues if there is a race condition :-)
Quoted 3 lines
Also pls note that the open source
driver
Supports only one MSI-X for now and this problem will not occur.
Are you referring to the kernel driver or the adaptec driver? Both are open source...
Anyway, I'm pretty sure I saw interrupts with different vectors arriving with the kernel driver.
Quoted 11 lines
Quoted 5 lines
BTW, another difference between the linux kernel driver and the adaptec version are several of the defines in
pm8001_defs.h: e.g.MPI_QUEUEis 256 in the adaptec driver, while it is 1024 in the kerneldriver. There are other differences as well.
Quoted 1 line
Different value may reflect different performance character, but both should works, there is no one for all setting.
Anand >> I agree with Jack. I will check on the #defines and get back.
Thanks!
Quoted 4 lines
Quoted 2 lines
Are all the changes in the kernel correct? I would like to have a confirmation of that before I am going to trust my data to this
driver.
Quoted 1 line
Quoted 8 lines
Quoted 2 lines
It clearly hasn't been tested with actual hardware :-(
:_(
Quoted 1 line
Quoted 1 line
My sincere apologies. I tested the same with single controller and it
worked fine. However I messed up when submitting
The patches. This was my first open source submission and request you to bear the inconvenience.
No problem, such things happen. Luckily I'm a kernel developer so I know my way around a driver, even though scsi is not my area of expertise.
Note that I saw one other difference between the kernel and adaptec driver:
the adaptec driver has support for SGPIO commands. I'm not sure what it does or whether it affects the pm80xx devices.
Regards,
Hans