* RE: [PATCH 3/5] drivers/infiniband: correct size computation [not found] ` <Pine.LNX.4.64.0912061015540.20858-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org> @ 2009-12-07 16:17 ` Tung, Chien Tin 0 siblings, 0 replies; 8+ messages in thread From: Tung, Chien Tin @ 2009-12-07 16:17 UTC (permalink / raw) To: Julia Lawall, Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock Thanks for pointing out the bug. > /* Remap the PCI registers in adapter BAR0 to kernel VA space */ >- mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); >+ mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), >+ sizeof(*mmio_regs)); > if (mmio_regs == NULL) { > printk(KERN_ERR PFX "Unable to remap BAR0\n"); > ret = -EIO; mmio_regs is initialized to NULL at the top of the function so *mmio_regs wouldn't be a good idea. Instead of sizeof(*mmio_regs) use pci_resource_len(pcidev, BAR_0). If you can recreate the patch with this change I will ack it. Thanks, Chien -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <603F8A3875DCE940BA37B49D0A6EA0AE3CE36BF4@azsmsx501.amr.corp.intel.com>]
[parent not found: <603F8A3875DCE940BA37B49D0A6EA0AE3CE36BF4-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* RE: [PATCH 3/5] drivers/infiniband: correct size computation [not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36BF4-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2009-12-07 16:22 ` Julia Lawall 2009-12-07 16:38 ` Tung, Chien Tin 0 siblings, 1 reply; 8+ messages in thread From: Julia Lawall @ 2009-12-07 16:22 UTC (permalink / raw) To: Tung, Chien Tin Cc: Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Mon, 7 Dec 2009, Tung, Chien Tin wrote: > Thanks for pointing out the bug. > > > /* Remap the PCI registers in adapter BAR0 to kernel VA space */ > >- mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); > >+ mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), > >+ sizeof(*mmio_regs)); > > if (mmio_regs == NULL) { > > printk(KERN_ERR PFX "Unable to remap BAR0\n"); > > ret = -EIO; > > mmio_regs is initialized to NULL at the top of the function so > *mmio_regs wouldn't be a good idea. Instead of sizeof(*mmio_regs) use > pci_resource_len(pcidev, BAR_0). If you can recreate the patch with > this change I will ack it. When you say that it isn't a good idea, do you mean that the result is wrong, or that it looks odd? I didn't think sizeof looked at the value, but only at the type? julia -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 3/5] drivers/infiniband: correct size computation 2009-12-07 16:22 ` Julia Lawall @ 2009-12-07 16:38 ` Tung, Chien Tin [not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36C9F-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 8+ messages in thread From: Tung, Chien Tin @ 2009-12-07 16:38 UTC (permalink / raw) To: Julia Lawall Cc: Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org >> > /* Remap the PCI registers in adapter BAR0 to kernel VA space */ >> >- mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); >> >+ mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), >> >+ sizeof(*mmio_regs)); >> > if (mmio_regs == NULL) { >> > printk(KERN_ERR PFX "Unable to remap BAR0\n"); >> > ret = -EIO; >> >> mmio_regs is initialized to NULL at the top of the function so >> *mmio_regs wouldn't be a good idea. Instead of sizeof(*mmio_regs) use >> pci_resource_len(pcidev, BAR_0). If you can recreate the patch with >> this change I will ack it. > >When you say that it isn't a good idea, do you mean that the result is >wrong, or that it looks odd? I didn't think sizeof looked at the value, >but only at the type? I misspoke on the problem. Mmio_regs is declared as a void pointer: void __iomem *mmio_regs = NULL; Thus pci_resource_len(pcidev, BAR_0) is the correct fix. Chien ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <603F8A3875DCE940BA37B49D0A6EA0AE3CE36C9F-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* RE: [PATCH 3/5] drivers/infiniband: correct size computation [not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36C9F-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2009-12-07 16:42 ` Julia Lawall 2009-12-07 19:57 ` Julia Lawall 1 sibling, 0 replies; 8+ messages in thread From: Julia Lawall @ 2009-12-07 16:42 UTC (permalink / raw) To: Tung, Chien Tin Cc: Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On Mon, 7 Dec 2009, Tung, Chien Tin wrote: > >> > /* Remap the PCI registers in adapter BAR0 to kernel VA space */ > >> >- mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); > >> >+ mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), > >> >+ sizeof(*mmio_regs)); > >> > if (mmio_regs == NULL) { > >> > printk(KERN_ERR PFX "Unable to remap BAR0\n"); > >> > ret = -EIO; > >> > >> mmio_regs is initialized to NULL at the top of the function so > >> *mmio_regs wouldn't be a good idea. Instead of sizeof(*mmio_regs) use > >> pci_resource_len(pcidev, BAR_0). If you can recreate the patch with > >> this change I will ack it. > > > >When you say that it isn't a good idea, do you mean that the result is > >wrong, or that it looks odd? I didn't think sizeof looked at the value, > >but only at the type? > > > I misspoke on the problem. Mmio_regs is declared as a void pointer: > > void __iomem *mmio_regs = NULL; > > Thus pci_resource_len(pcidev, BAR_0) is the correct fix. OK, thanks for the clarification. I will make another patch later today. julia -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 3/5] drivers/infiniband: correct size computation [not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36C9F-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org> 2009-12-07 16:42 ` Julia Lawall @ 2009-12-07 19:57 ` Julia Lawall 2009-12-07 20:09 ` Tung, Chien Tin [not found] ` <Pine.LNX.4.64.0912072056160.1202-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org> 1 sibling, 2 replies; 8+ messages in thread From: Julia Lawall @ 2009-12-07 19:57 UTC (permalink / raw) To: Tung, Chien Tin Cc: Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kernel-janitors-u79uwXL29TY76Z2rM5mHXA@public.gmane.org From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org> The size argument to ioremap_nocache should be the size of desired information, not the pointer to it. The semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // <smpl> @expression@ expression *x; @@ x = <+... *sizeof(x) ...+>// </smpl> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org> --- drivers/infiniband/hw/nes/nes.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c index cbde0cf..95db98f 100644 --- a/drivers/infiniband/hw/nes/nes.c +++ b/drivers/infiniband/hw/nes/nes.c @@ -521,7 +521,8 @@ static int __devinit nes_probe(struct pci_dev *pcidev, const struct pci_device_i spin_lock_init(&nesdev->indexed_regs_lock); /* Remap the PCI registers in adapter BAR0 to kernel VA space */ - mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); + mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), + pci_resource_len(pcidev, BAR_0)); if (mmio_regs == NULL) { printk(KERN_ERR PFX "Unable to remap BAR0\n"); ret = -EIO; -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 3/5] drivers/infiniband: correct size computation 2009-12-07 19:57 ` Julia Lawall @ 2009-12-07 20:09 ` Tung, Chien Tin [not found] ` <Pine.LNX.4.64.0912072056160.1202-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org> 1 sibling, 0 replies; 8+ messages in thread From: Tung, Chien Tin @ 2009-12-07 20:09 UTC (permalink / raw) To: Julia Lawall Cc: Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org >From: Julia Lawall <julia@diku.dk> > >The size argument to ioremap_nocache should be the size of desired >information, not the pointer to it. > >The semantic match that finds this problem is as follows: >(http://coccinelle.lip6.fr/) > >// <smpl> >@expression@ >expression *x; >@@ > >x = > <+... >*sizeof(x) >...+>// </smpl> > >Signed-off-by: Julia Lawall <julia@diku.dk> > >--- > drivers/infiniband/hw/nes/nes.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > >diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c >index cbde0cf..95db98f 100644 >--- a/drivers/infiniband/hw/nes/nes.c >+++ b/drivers/infiniband/hw/nes/nes.c >@@ -521,7 +521,8 @@ static int __devinit nes_probe(struct pci_dev *pcidev, const struct pci_device_i > spin_lock_init(&nesdev->indexed_regs_lock); > > /* Remap the PCI registers in adapter BAR0 to kernel VA space */ >- mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); >+ mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), >+ pci_resource_len(pcidev, BAR_0)); > if (mmio_regs == NULL) { > printk(KERN_ERR PFX "Unable to remap BAR0\n"); > ret = -EIO; Acked-by: Chien Tung <chien.tin.tung@intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <Pine.LNX.4.64.0912072056160.1202-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>]
* Re: [PATCH 3/5] drivers/infiniband: correct size computation [not found] ` <Pine.LNX.4.64.0912072056160.1202-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org> @ 2009-12-09 22:32 ` Roland Dreier 0 siblings, 0 replies; 8+ messages in thread From: Roland Dreier @ 2009-12-09 22:32 UTC (permalink / raw) To: Julia Lawall Cc: Tung, Chien Tin, Latif, Faisal, Roland Dreier, Hefty, Sean, Hal Rosenstock, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org applied, thanks everyone. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/5] drivers/infiniband: correct size computation @ 2009-12-06 9:16 Julia Lawall 0 siblings, 0 replies; 8+ messages in thread From: Julia Lawall @ 2009-12-06 9:16 UTC (permalink / raw) To: Faisal Latif, Chien Tung, Roland Dreier, Sean Hefty, Hal Rosenstock From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org> The size argument to ioremap_nocache should be the size of desired information, not the pointer to it. The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/) // <smpl> @expression@ expression *x; @@ x = <+... -sizeof(x) +sizeof(*x) ...+>// </smpl> Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org> --- drivers/infiniband/hw/nes/nes.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c index cbde0cf..b8c5372 100644 --- a/drivers/infiniband/hw/nes/nes.c +++ b/drivers/infiniband/hw/nes/nes.c @@ -521,7 +521,8 @@ static int __devinit nes_probe(struct pci_dev *pcidev, const struct pci_device_i spin_lock_init(&nesdev->indexed_regs_lock); /* Remap the PCI registers in adapter BAR0 to kernel VA space */ - mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), sizeof(mmio_regs)); + mmio_regs = ioremap_nocache(pci_resource_start(pcidev, BAR_0), + sizeof(*mmio_regs)); if (mmio_regs == NULL) { printk(KERN_ERR PFX "Unable to remap BAR0\n"); ret = -EIO; -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-12-09 22:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <Pine.LNX.4.64.0912061015540.20858@ask.diku.dk>
[not found] ` <Pine.LNX.4.64.0912061015540.20858-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
2009-12-07 16:17 ` [PATCH 3/5] drivers/infiniband: correct size computation Tung, Chien Tin
[not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36BF4@azsmsx501.amr.corp.intel.com>
[not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36BF4-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2009-12-07 16:22 ` Julia Lawall
2009-12-07 16:38 ` Tung, Chien Tin
[not found] ` <603F8A3875DCE940BA37B49D0A6EA0AE3CE36C9F-uLM7Qlg6Mbekrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2009-12-07 16:42 ` Julia Lawall
2009-12-07 19:57 ` Julia Lawall
2009-12-07 20:09 ` Tung, Chien Tin
[not found] ` <Pine.LNX.4.64.0912072056160.1202-QfmoRoYWmW9knbxzx/v8hQ@public.gmane.org>
2009-12-09 22:32 ` Roland Dreier
2009-12-06 9:16 Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox