Re: [Intel-wired-lan] [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On Mon, Jul 6, 2015 at 12:31 PM, Rustad, Mark D mark.d.rus...@intel.com wrote: On Jun 26, 2015, at 11:04 AM, Rustad, Mark D mark.d.rus...@intel.com wrote: Sorry, Mark, I've just been busy with other issues and haven't had a chance to look at this yet. Is there any chance of this getting into this merge window? Well, it has missed the merge window, but this really is a bug fix. These patches address problems that, under race conditions, can corrupt VPD data and under other conditions can cause hangs. In fact I would submit that the reason that the VPD operations have been made interruptible is directly related to hangs caused by the sharing of VPD capability registers between functions. You see, if one function ever performs a VPD write, any subsequent read on any other function that shares those registers will definitely hang. As a rule, after the merge window closes, I only merge fixes for things we broke during the merge window or drivers for brand-new things, where there's no chance of breaking something that used to work. It would be nice to have a description of what a user might see when tripping over this problem and maybe some pointers to problem reports. I see it fixes concurrent access problems, and I infer that it's related to sysfs. Feel free to add a stable tag when you post your patches. I'll try to remember to add it when merging this. When it's relevant, it's nice to include the SHA1 of the commit that introduced the bug. But in this case, I think it's been there forever. Bjorn -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On Jun 26, 2015, at 11:04 AM, Rustad, Mark D mark.d.rus...@intel.com wrote: Sorry, Mark, I've just been busy with other issues and haven't had a chance to look at this yet. Is there any chance of this getting into this merge window? Well, it has missed the merge window, but this really is a bug fix. These patches address problems that, under race conditions, can corrupt VPD data and under other conditions can cause hangs. In fact I would submit that the reason that the VPD operations have been made interruptible is directly related to hangs caused by the sharing of VPD capability registers between functions. You see, if one function ever performs a VPD write, any subsequent read on any other function that shares those registers will definitely hang. I imagine that there are many devices beyond Intel's Ethernet devices that would benefit from using the quirk that these patches introduce. Please apply it and consider it for -stable. -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On Jun 17, 2015, at 9:44 AM, Bjorn Helgaas bhelg...@google.com wrote: On Wed, Jun 17, 2015 at 11:29 AM, Rustad, Mark D mark.d.rus...@intel.com wrote: + Alex On Jun 5, 2015, at 2:59 PM, Rustad, Mark D mark.d.rus...@intel.com wrote: On Jun 3, 2015, at 11:46 AM, Mark D Rustad mark.d.rus...@intel.com wrote: Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch So does this series look acceptable now? I think I addressed the issues that Alex raised. Can these also be considered for -stable? More than a week has passed without any comment. Is this going to be accepted or is there still an issue? Sorry, Mark, I've just been busy with other issues and haven't had a chance to look at this yet. Is there any chance of this getting into this merge window? -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On 06/17/2015 09:29 AM, Rustad, Mark D wrote: + Alex On Jun 5, 2015, at 2:59 PM, Rustad, Mark D mark.d.rus...@intel.com wrote: On Jun 3, 2015, at 11:46 AM, Mark D Rustad mark.d.rus...@intel.com wrote: Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch So does this series look acceptable now? I think I addressed the issues that Alex raised. Can these also be considered for -stable? More than a week has passed without any comment. Is this going to be accepted or is there still an issue? Yeah, this looks like it has addressed most of the corner cases so I am good with it. Acked-by: Alexander Duyck alexander.h.du...@redhat.com -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On Wed, Jun 17, 2015 at 11:29 AM, Rustad, Mark D mark.d.rus...@intel.com wrote: + Alex On Jun 5, 2015, at 2:59 PM, Rustad, Mark D mark.d.rus...@intel.com wrote: On Jun 3, 2015, at 11:46 AM, Mark D Rustad mark.d.rus...@intel.com wrote: Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch So does this series look acceptable now? I think I addressed the issues that Alex raised. Can these also be considered for -stable? More than a week has passed without any comment. Is this going to be accepted or is there still an issue? Sorry, Mark, I've just been busy with other issues and haven't had a chance to look at this yet. Bjorn -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
+ Alex On Jun 5, 2015, at 2:59 PM, Rustad, Mark D mark.d.rus...@intel.com wrote: On Jun 3, 2015, at 11:46 AM, Mark D Rustad mark.d.rus...@intel.com wrote: Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch So does this series look acceptable now? I think I addressed the issues that Alex raised. Can these also be considered for -stable? More than a week has passed without any comment. Is this going to be accepted or is there still an issue? -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
On Jun 3, 2015, at 11:46 AM, Mark D Rustad mark.d.rus...@intel.com wrote: Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch So does this series look acceptable now? I think I addressed the issues that Alex raised. Can these also be considered for -stable? -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
[PATCH V3 0/2] pci: Provide a flag to access VPD through function 0
Many multi-function devices provide shared registers in extended config space for accessing VPD. The behavior of these registers means that the state must be tracked and access locked correctly for accesses not to hang or worse. One way to meet these needs is to always perform the accesses through function 0, thereby using the state tracking and mutex that already exists. To provide this behavior, add a dev_flags bit to indicate that this should be done. This bit can then be set for any non-zero function that needs to redirect such VPD access to function 0. Do not set this bit on the zero function or there will be an infinite recursion. The second patch uses this new flag to invoke this behavior on all multi-function Intel Ethernet devices. Signed-off-by: Mark Rustad mark.d.rus...@intel.com --- Changes in V2: - Corrected a spelling error in a log message - Added checks to see that the referenced function 0 is reasonable Changes in V3: - Don't leak a device reference - Check that function 0 has VPD - Make a helper for the function 0 checks - Moved a multifunction check to the quirk patch --- Mark D Rustad (2): pci: Add dev_flags bit to access VPD through function 0 pci: Add VPD quirk for Intel Ethernet devices drivers/pci/access.c | 61 +- drivers/pci/quirks.c |9 +++ 2 files changed, 69 insertions(+), 1 deletion(-) -- Mark Rustad, Network Division, Intel Corporation -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
