Message ID | 20140331035439.GA11288@richard |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Mon, Mar 31, 2014 at 6:54 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > > On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: > >On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: > >> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass > >> pci_device_id.driver_data to __mlx4_init_one during reset". > >> > >> pci_match_id() might return NULL if someone binds the driver to a device > >> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' > >> before using it. > >> > >> Thanks to Bjorn who raised the problem. > > > >Well, that commit was applied to net and is now present in Linus > >tree... so assuming it's too late for 3.14, need to queue this for > >-stable > > > >Or. > > Sorry for this bothering, hope this will not block someone. > > Here is my suggestion for fixing this, not sure this is a good way to export > pci_match_device() to modules. This is my current solution to this problem. If > you have any comments, please let me know. > > ------------------------------------------------------------------------------ > From 9361e1edd6776202c6e11dd44d3d4d72c990b111 Mon Sep 17 00:00:00 2001 > From: Wei Yang <weiyang@linux.vnet.ibm.com> > Date: Mon, 31 Mar 2014 11:34:57 +0800 > Subject: [PATCH net-next] net/mlx4_core: match pci_device_id including dynids Your original commit went to net and same needs to be done for the fix > > > Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass > pci_device_id.driver_data to __mlx4_init_one during reset". > > pci_match_id() just match the static pci_device_id, which may return NULL if > someone binds the driver to a device manually using > /sys/bus/pci/drivers/.../new_id. > > This patch match pci_device_id with pci_match_device() to cover both dynids > and static id_table. > > Thanks to Bjorn finding this issue. > > CC: Bjorn Helgaas <bhelgaas@google.com> > CC: Amir Vadai <amirv@mellanox.com> > Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> > --- > drivers/net/ethernet/mellanox/mlx4/main.c | 4 +++- > drivers/pci/pci-driver.c | 3 ++- > include/linux/pci.h | 2 ++ > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c > index aa54ef7..b0edb5c 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/main.c > +++ b/drivers/net/ethernet/mellanox/mlx4/main.c > @@ -2673,7 +2673,9 @@ static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev) > const struct pci_device_id *id; > int ret; > > - id = pci_match_id(mlx4_pci_table, pdev); > + id = pci_match_device(pci_dev_driver(pdev), pdev); > + BUG_ON(!id); > + > ret = __mlx4_init_one(pdev, id->driver_data); > > return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 25f0bc6..1ee26a1 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -225,7 +225,7 @@ const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, > * system is in its list of supported devices. Returns the matching > * pci_device_id structure or %NULL if there is no match. > */ > -static const struct pci_device_id *pci_match_device(struct pci_driver *drv, > +const struct pci_device_id *pci_match_device(struct pci_driver *drv, > struct pci_dev *dev) > { > struct pci_dynid *dynid; > @@ -1355,6 +1355,7 @@ postcore_initcall(pci_driver_init); > > EXPORT_SYMBOL_GPL(pci_add_dynid); > EXPORT_SYMBOL(pci_match_id); > +EXPORT_SYMBOL(pci_match_device); > EXPORT_SYMBOL(__pci_register_driver); > EXPORT_SYMBOL(pci_unregister_driver); > EXPORT_SYMBOL(pci_dev_driver); > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 6d1cc9e..d7a7d05 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1135,6 +1135,8 @@ int pci_add_dynid(struct pci_driver *drv, > unsigned long driver_data); > const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, > struct pci_dev *dev); > +const struct pci_device_id *pci_match_device(struct pci_driver *drv, > + struct pci_dev *dev); > int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, > int pass); > > -- > 1.7.9.5 > > > -- > Richard Yang > Help you, Help me > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/31/2014 7:52 AM, Or Gerlitz wrote: > On Mon, Mar 31, 2014 at 6:54 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: >> >> On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >>> On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >>>> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >>>> pci_device_id.driver_data to __mlx4_init_one during reset". >>>> >>>> pci_match_id() might return NULL if someone binds the driver to a device >>>> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >>>> before using it. >>>> >>>> Thanks to Bjorn who raised the problem. >>> >>> Well, that commit was applied to net and is now present in Linus >>> tree... so assuming it's too late for 3.14, need to queue this for >>> -stable >>> >>> Or. >> >> Sorry for this bothering, hope this will not block someone. >> >> Here is my suggestion for fixing this, not sure this is a good way to export >> pci_match_device() to modules. This is my current solution to this problem. If >> you have any comments, please let me know. >> >> ------------------------------------------------------------------------------ >> From 9361e1edd6776202c6e11dd44d3d4d72c990b111 Mon Sep 17 00:00:00 2001 >> From: Wei Yang <weiyang@linux.vnet.ibm.com> >> Date: Mon, 31 Mar 2014 11:34:57 +0800 >> Subject: [PATCH net-next] net/mlx4_core: match pci_device_id including dynids > > > > Your original commit went to net and same needs to be done for the fix > > > >> >> >> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >> pci_device_id.driver_data to __mlx4_init_one during reset". >> >> pci_match_id() just match the static pci_device_id, which may return NULL if >> someone binds the driver to a device manually using >> /sys/bus/pci/drivers/.../new_id. >> >> This patch match pci_device_id with pci_match_device() to cover both dynids >> and static id_table. >> >> Thanks to Bjorn finding this issue. >> >> CC: Bjorn Helgaas <bhelgaas@google.com> >> CC: Amir Vadai <amirv@mellanox.com> >> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com> Acked-By: Amir Vadai <amirv@mellanox.com> And of-course need to fix the net/net-next/stable thing -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 31, 2014 at 07:52:20AM +0300, Or Gerlitz wrote: >On Mon, Mar 31, 2014 at 6:54 AM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: >> >> On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >> >On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >> >> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >> >> pci_device_id.driver_data to __mlx4_init_one during reset". >> >> >> >> pci_match_id() might return NULL if someone binds the driver to a device >> >> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >> >> before using it. >> >> >> >> Thanks to Bjorn who raised the problem. >> > >> >Well, that commit was applied to net and is now present in Linus >> >tree... so assuming it's too late for 3.14, need to queue this for >> >-stable >> > >> >Or. >> >> Sorry for this bothering, hope this will not block someone. >> >> Here is my suggestion for fixing this, not sure this is a good way to export >> pci_match_device() to modules. This is my current solution to this problem. If >> you have any comments, please let me know. >> >> ------------------------------------------------------------------------------ >> From 9361e1edd6776202c6e11dd44d3d4d72c990b111 Mon Sep 17 00:00:00 2001 >> From: Wei Yang <weiyang@linux.vnet.ibm.com> >> Date: Mon, 31 Mar 2014 11:34:57 +0800 >> Subject: [PATCH net-next] net/mlx4_core: match pci_device_id including dynids > > > >Your original commit went to net and same needs to be done for the fix > Thanks, seems I still misunderstand this rule. I thought Amir is correct, so copyed his. Will pay attention next time.
From: Wei Yang <weiyang@linux.vnet.ibm.com> Date: Mon, 31 Mar 2014 11:54:39 +0800 > On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >>On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >>> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >>> pci_device_id.driver_data to __mlx4_init_one during reset". >>> >>> pci_match_id() might return NULL if someone binds the driver to a device >>> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >>> before using it. >>> >>> Thanks to Bjorn who raised the problem. >> >>Well, that commit was applied to net and is now present in Linus >>tree... so assuming it's too late for 3.14, need to queue this for >>-stable >> >>Or. > > Sorry for this bothering, hope this will not block someone. > > Here is my suggestion for fixing this, not sure this is a good way to export > pci_match_device() to modules. This is my current solution to this problem. If > you have any comments, please let me know. This needs to be ACK'd by the PCI maintainers, please make sure they see this. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 31, 2014 at 04:32:40PM -0400, David Miller wrote: >From: Wei Yang <weiyang@linux.vnet.ibm.com> >Date: Mon, 31 Mar 2014 11:54:39 +0800 > >> On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >>>On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >>>> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >>>> pci_device_id.driver_data to __mlx4_init_one during reset". >>>> >>>> pci_match_id() might return NULL if someone binds the driver to a device >>>> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >>>> before using it. >>>> >>>> Thanks to Bjorn who raised the problem. >>> >>>Well, that commit was applied to net and is now present in Linus >>>tree... so assuming it's too late for 3.14, need to queue this for >>>-stable >>> >>>Or. >> >> Sorry for this bothering, hope this will not block someone. >> >> Here is my suggestion for fixing this, not sure this is a good way to export >> pci_match_device() to modules. This is my current solution to this problem. If >> you have any comments, please let me know. > >This needs to be ACK'd by the PCI maintainers, please make sure they see this. Yes, I think Bjorn, maintainers of PCI, is in the list. Bjorn, Do you have some concern on this implementation? Or, you suggest me to send this to pci maillist too?
On Mon, Mar 31, 2014 at 7:41 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: > On Mon, Mar 31, 2014 at 04:32:40PM -0400, David Miller wrote: >>From: Wei Yang <weiyang@linux.vnet.ibm.com> >>Date: Mon, 31 Mar 2014 11:54:39 +0800 >> >>> On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >>>>On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >>>>> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >>>>> pci_device_id.driver_data to __mlx4_init_one during reset". >>>>> >>>>> pci_match_id() might return NULL if someone binds the driver to a device >>>>> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >>>>> before using it. >>>>> >>>>> Thanks to Bjorn who raised the problem. >>>> >>>>Well, that commit was applied to net and is now present in Linus >>>>tree... so assuming it's too late for 3.14, need to queue this for >>>>-stable >>>> >>>>Or. >>> >>> Sorry for this bothering, hope this will not block someone. >>> >>> Here is my suggestion for fixing this, not sure this is a good way to export >>> pci_match_device() to modules. This is my current solution to this problem. If >>> you have any comments, please let me know. >> >>This needs to be ACK'd by the PCI maintainers, please make sure they see this. > > Yes, I think Bjorn, maintainers of PCI, is in the list. > > Bjorn, > > Do you have some concern on this implementation? > Or, you suggest me to send this to pci maillist too? I haven't looked at the patch yet, but yes, please do copy linux-pci as well. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 31, 2014 at 09:12:04PM -0600, Bjorn Helgaas wrote: >On Mon, Mar 31, 2014 at 7:41 PM, Wei Yang <weiyang@linux.vnet.ibm.com> wrote: >> On Mon, Mar 31, 2014 at 04:32:40PM -0400, David Miller wrote: >>>From: Wei Yang <weiyang@linux.vnet.ibm.com> >>>Date: Mon, 31 Mar 2014 11:54:39 +0800 >>> >>>> On Sun, Mar 30, 2014 at 09:08:06PM +0300, Or Gerlitz wrote: >>>>>On Sun, Mar 30, 2014 at 6:26 PM, Amir Vadai <amirv@mellanox.com> wrote: >>>>>> Fix issue introduced by commit: 97a5221 "net/mlx4_core: pass >>>>>> pci_device_id.driver_data to __mlx4_init_one during reset". >>>>>> >>>>>> pci_match_id() might return NULL if someone binds the driver to a device >>>>>> manually using /sys/bus/pci/drivers/.../new_id. Need to check 'id' >>>>>> before using it. >>>>>> >>>>>> Thanks to Bjorn who raised the problem. >>>>> >>>>>Well, that commit was applied to net and is now present in Linus >>>>>tree... so assuming it's too late for 3.14, need to queue this for >>>>>-stable >>>>> >>>>>Or. >>>> >>>> Sorry for this bothering, hope this will not block someone. >>>> >>>> Here is my suggestion for fixing this, not sure this is a good way to export >>>> pci_match_device() to modules. This is my current solution to this problem. If >>>> you have any comments, please let me know. >>> >>>This needs to be ACK'd by the PCI maintainers, please make sure they see this. >> >> Yes, I think Bjorn, maintainers of PCI, is in the list. >> >> Bjorn, >> >> Do you have some concern on this implementation? >> Or, you suggest me to send this to pci maillist too? > >I haven't looked at the patch yet, but yes, please do copy linux-pci as well. Ok, I will send this one again to both net and pci mail list.
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c index aa54ef7..b0edb5c 100644 --- a/drivers/net/ethernet/mellanox/mlx4/main.c +++ b/drivers/net/ethernet/mellanox/mlx4/main.c @@ -2673,7 +2673,9 @@ static pci_ers_result_t mlx4_pci_slot_reset(struct pci_dev *pdev) const struct pci_device_id *id; int ret; - id = pci_match_id(mlx4_pci_table, pdev); + id = pci_match_device(pci_dev_driver(pdev), pdev); + BUG_ON(!id); + ret = __mlx4_init_one(pdev, id->driver_data); return ret ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED; diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 25f0bc6..1ee26a1 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -225,7 +225,7 @@ const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, * system is in its list of supported devices. Returns the matching * pci_device_id structure or %NULL if there is no match. */ -static const struct pci_device_id *pci_match_device(struct pci_driver *drv, +const struct pci_device_id *pci_match_device(struct pci_driver *drv, struct pci_dev *dev) { struct pci_dynid *dynid; @@ -1355,6 +1355,7 @@ postcore_initcall(pci_driver_init); EXPORT_SYMBOL_GPL(pci_add_dynid); EXPORT_SYMBOL(pci_match_id); +EXPORT_SYMBOL(pci_match_device); EXPORT_SYMBOL(__pci_register_driver); EXPORT_SYMBOL(pci_unregister_driver); EXPORT_SYMBOL(pci_dev_driver); diff --git a/include/linux/pci.h b/include/linux/pci.h index 6d1cc9e..d7a7d05 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1135,6 +1135,8 @@ int pci_add_dynid(struct pci_driver *drv, unsigned long driver_data); const struct pci_device_id *pci_match_id(const struct pci_device_id *ids, struct pci_dev *dev); +const struct pci_device_id *pci_match_device(struct pci_driver *drv, + struct pci_dev *dev); int pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max, int pass);