| Message ID | 20170725205753.4735-7-logang@deltatee.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Tue, Jul 25, 2017 at 02:57:43PM -0600, Logan Gunthorpe wrote: > With Switchtec hardware it's impossible to get the alignment parameters > for a peer's memory window until the peer's driver has configured it's s/it's/its/ > windows. Strictly speaking, the link doesn't have to be up for this, > but the link being up is the only way the client can tell that > the otherside has been configured. s/otherside/other side/ > This patch converts ntb_transport and ntb_perf to use this function after > the link goes up. This simplifies these clients slightly because they > no longer have to store the alignment parameters. It also tweaks > ntb_tool so that peer_mw_trans will print zero if it is run before > the link goes up. > > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> > --- > drivers/ntb/ntb_transport.c | 20 ++++++++++---------- > drivers/ntb/test/ntb_perf.c | 18 +++++++++--------- > drivers/ntb/test/ntb_tool.c | 6 +++--- > 3 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c > index b29558ddfe95..7ed745aaa213 100644 > --- a/drivers/ntb/ntb_transport.c > +++ b/drivers/ntb/ntb_transport.c > @@ -191,8 +191,6 @@ struct ntb_transport_qp { > struct ntb_transport_mw { > phys_addr_t phys_addr; > resource_size_t phys_size; > - resource_size_t xlat_align; > - resource_size_t xlat_align_size; > void __iomem *vbase; > size_t xlat_size; > size_t buff_size; > @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, > struct ntb_transport_mw *mw = &nt->mw_vec[num_mw]; > struct pci_dev *pdev = nt->ndev->pdev; > size_t xlat_size, buff_size; > + resource_size_t xlat_align; > + resource_size_t xlat_align_size; > int rc; > > if (!size) > return -EINVAL; > > - xlat_size = round_up(size, mw->xlat_align_size); > - buff_size = round_up(size, mw->xlat_align); > + rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align, > + &xlat_align_size, NULL); > + if (rc) > + return rc; > + > + xlat_size = round_up(size, xlat_align_size); > + buff_size = round_up(size, xlat_align); > > /* No need to re-setup */ > if (mw->xlat_size == xlat_size) > @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, > * is a requirement of the hardware. It is recommended to setup CMA > * for BAR sizes equal or greater than 4MB. > */ > - if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) { > + if (!IS_ALIGNED(mw->dma_addr, xlat_align)) { > dev_err(&pdev->dev, "DMA memory %pad is not aligned\n", > &mw->dma_addr); > ntb_free_mw(nt, num_mw); > @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) > for (i = 0; i < mw_count; i++) { > mw = &nt->mw_vec[i]; > > - rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align, > - &mw->xlat_align_size, NULL); > - if (rc) > - goto err1; > - > rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr, > &mw->phys_size); > if (rc) > diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c > index 759f772fa00c..427112cf101a 100644 > --- a/drivers/ntb/test/ntb_perf.c > +++ b/drivers/ntb/test/ntb_perf.c > @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)") > struct perf_mw { > phys_addr_t phys_addr; > resource_size_t phys_size; > - resource_size_t xlat_align; > - resource_size_t xlat_align_size; > void __iomem *vbase; > size_t xlat_size; > size_t buf_size; > @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size) > { > struct perf_mw *mw = &perf->mw; > size_t xlat_size, buf_size; > + resource_size_t xlat_align; > + resource_size_t xlat_align_size; > int rc; > > if (!size) > return -EINVAL; > > - xlat_size = round_up(size, mw->xlat_align_size); > - buf_size = round_up(size, mw->xlat_align); > + rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align, > + &xlat_align_size, NULL); > + if (rc) > + return rc; > + > + xlat_size = round_up(size, xlat_align_size); > + buf_size = round_up(size, xlat_align); > > if (mw->xlat_size == xlat_size) > return 0; > @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf) > > mw = &perf->mw; > > - rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align, > - &mw->xlat_align_size, NULL); > - if (rc) > - return rc; > - > rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size); > if (rc) > return rc; > diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c > index a69815c45ce6..91526a986caa 100644 > --- a/drivers/ntb/test/ntb_tool.c > +++ b/drivers/ntb/test/ntb_tool.c > @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep, > > phys_addr_t base; > resource_size_t mw_size; > - resource_size_t align_addr; > - resource_size_t align_size; > - resource_size_t max_size; > + resource_size_t align_addr = 0; > + resource_size_t align_size = 0; > + resource_size_t max_size = 0; > > buf_size = min_t(size_t, size, 512); > > -- > 2.11.0 >
On Tue, Jul 25, 2017 at 02:57:43PM -0600, Logan Gunthorpe wrote: > With Switchtec hardware it's impossible to get the alignment parameters > for a peer's memory window until the peer's driver has configured it's > windows. Strictly speaking, the link doesn't have to be up for this, > but the link being up is the only way the client can tell that > the otherside has been configured. > > This patch converts ntb_transport and ntb_perf to use this function after > the link goes up. This simplifies these clients slightly because they > no longer have to store the alignment parameters. It also tweaks > ntb_tool so that peer_mw_trans will print zero if it is run before > the link goes up. > > Signed-off-by: Logan Gunthorpe <logang@deltatee.com> This also looks like a bug fix. However, given that it is a little more invasive and no one reporting any problems with it, I'm less apt to pull it into the bug-fixes branch without anyone compilaining of a bug (lest I break something in -stable). Thanks, Jon > --- > drivers/ntb/ntb_transport.c | 20 ++++++++++---------- > drivers/ntb/test/ntb_perf.c | 18 +++++++++--------- > drivers/ntb/test/ntb_tool.c | 6 +++--- > 3 files changed, 22 insertions(+), 22 deletions(-) > > diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c > index b29558ddfe95..7ed745aaa213 100644 > --- a/drivers/ntb/ntb_transport.c > +++ b/drivers/ntb/ntb_transport.c > @@ -191,8 +191,6 @@ struct ntb_transport_qp { > struct ntb_transport_mw { > phys_addr_t phys_addr; > resource_size_t phys_size; > - resource_size_t xlat_align; > - resource_size_t xlat_align_size; > void __iomem *vbase; > size_t xlat_size; > size_t buff_size; > @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, > struct ntb_transport_mw *mw = &nt->mw_vec[num_mw]; > struct pci_dev *pdev = nt->ndev->pdev; > size_t xlat_size, buff_size; > + resource_size_t xlat_align; > + resource_size_t xlat_align_size; > int rc; > > if (!size) > return -EINVAL; > > - xlat_size = round_up(size, mw->xlat_align_size); > - buff_size = round_up(size, mw->xlat_align); > + rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align, > + &xlat_align_size, NULL); > + if (rc) > + return rc; > + > + xlat_size = round_up(size, xlat_align_size); > + buff_size = round_up(size, xlat_align); > > /* No need to re-setup */ > if (mw->xlat_size == xlat_size) > @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, > * is a requirement of the hardware. It is recommended to setup CMA > * for BAR sizes equal or greater than 4MB. > */ > - if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) { > + if (!IS_ALIGNED(mw->dma_addr, xlat_align)) { > dev_err(&pdev->dev, "DMA memory %pad is not aligned\n", > &mw->dma_addr); > ntb_free_mw(nt, num_mw); > @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) > for (i = 0; i < mw_count; i++) { > mw = &nt->mw_vec[i]; > > - rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align, > - &mw->xlat_align_size, NULL); > - if (rc) > - goto err1; > - > rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr, > &mw->phys_size); > if (rc) > diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c > index 759f772fa00c..427112cf101a 100644 > --- a/drivers/ntb/test/ntb_perf.c > +++ b/drivers/ntb/test/ntb_perf.c > @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)") > struct perf_mw { > phys_addr_t phys_addr; > resource_size_t phys_size; > - resource_size_t xlat_align; > - resource_size_t xlat_align_size; > void __iomem *vbase; > size_t xlat_size; > size_t buf_size; > @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size) > { > struct perf_mw *mw = &perf->mw; > size_t xlat_size, buf_size; > + resource_size_t xlat_align; > + resource_size_t xlat_align_size; > int rc; > > if (!size) > return -EINVAL; > > - xlat_size = round_up(size, mw->xlat_align_size); > - buf_size = round_up(size, mw->xlat_align); > + rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align, > + &xlat_align_size, NULL); > + if (rc) > + return rc; > + > + xlat_size = round_up(size, xlat_align_size); > + buf_size = round_up(size, xlat_align); > > if (mw->xlat_size == xlat_size) > return 0; > @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf) > > mw = &perf->mw; > > - rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align, > - &mw->xlat_align_size, NULL); > - if (rc) > - return rc; > - > rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size); > if (rc) > return rc; > diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c > index a69815c45ce6..91526a986caa 100644 > --- a/drivers/ntb/test/ntb_tool.c > +++ b/drivers/ntb/test/ntb_tool.c > @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep, > > phys_addr_t base; > resource_size_t mw_size; > - resource_size_t align_addr; > - resource_size_t align_size; > - resource_size_t max_size; > + resource_size_t align_addr = 0; > + resource_size_t align_size = 0; > + resource_size_t max_size = 0; > > buf_size = min_t(size_t, size, 512); > > -- > 2.11.0 > > -- > You received this message because you are subscribed to the Google Groups "linux-ntb" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com. > To post to this group, send email to linux-ntb@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/20170725205753.4735-7-logang%40deltatee.com. > For more options, visit https://groups.google.com/d/optout.
Hey, On 01/08/17 12:14 PM, Jon Mason wrote: > This also looks like a bug fix. However, given that it is a little more > invasive and no one reporting any problems with it, I'm less apt to > pull it into the bug-fixes branch without anyone compilaining of a > bug (lest I break something in -stable). Yes, 6 and 7 change a semantic of that function. I wouldn't call them bug fixes. But you can't pull 7 without pulling 6, as 7 would break everything unless the change in 6 occurs. Thanks, Logan
On Tue, Aug 01, 2017 at 12:17:47PM -0600, Logan Gunthorpe wrote: > Hey, > > On 01/08/17 12:14 PM, Jon Mason wrote: > > This also looks like a bug fix. However, given that it is a little more > > invasive and no one reporting any problems with it, I'm less apt to > > pull it into the bug-fixes branch without anyone compilaining of a > > bug (lest I break something in -stable). > > Yes, 6 and 7 change a semantic of that function. I wouldn't call them > bug fixes. But you can't pull 7 without pulling 6, as 7 would break > everything unless the change in 6 occurs. Okay, then I'll hold off on 7 then. It wasn't clear to me, based on the patch descriptions, that this was fallout from the previous patch. Thanks for clearing this up for me. Thanks, Jon > > Thanks, > > Logan > > -- > You received this message because you are subscribed to the Google Groups "linux-ntb" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com. > To post to this group, send email to linux-ntb@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/c4235dc9-90d0-1e68-7c73-c2a5bec83712%40deltatee.com. > For more options, visit https://groups.google.com/d/optout.
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c index b29558ddfe95..7ed745aaa213 100644 --- a/drivers/ntb/ntb_transport.c +++ b/drivers/ntb/ntb_transport.c @@ -191,8 +191,6 @@ struct ntb_transport_qp { struct ntb_transport_mw { phys_addr_t phys_addr; resource_size_t phys_size; - resource_size_t xlat_align; - resource_size_t xlat_align_size; void __iomem *vbase; size_t xlat_size; size_t buff_size; @@ -687,13 +685,20 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, struct ntb_transport_mw *mw = &nt->mw_vec[num_mw]; struct pci_dev *pdev = nt->ndev->pdev; size_t xlat_size, buff_size; + resource_size_t xlat_align; + resource_size_t xlat_align_size; int rc; if (!size) return -EINVAL; - xlat_size = round_up(size, mw->xlat_align_size); - buff_size = round_up(size, mw->xlat_align); + rc = ntb_mw_get_align(nt->ndev, PIDX, num_mw, &xlat_align, + &xlat_align_size, NULL); + if (rc) + return rc; + + xlat_size = round_up(size, xlat_align_size); + buff_size = round_up(size, xlat_align); /* No need to re-setup */ if (mw->xlat_size == xlat_size) @@ -722,7 +727,7 @@ static int ntb_set_mw(struct ntb_transport_ctx *nt, int num_mw, * is a requirement of the hardware. It is recommended to setup CMA * for BAR sizes equal or greater than 4MB. */ - if (!IS_ALIGNED(mw->dma_addr, mw->xlat_align)) { + if (!IS_ALIGNED(mw->dma_addr, xlat_align)) { dev_err(&pdev->dev, "DMA memory %pad is not aligned\n", &mw->dma_addr); ntb_free_mw(nt, num_mw); @@ -1106,11 +1111,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev) for (i = 0; i < mw_count; i++) { mw = &nt->mw_vec[i]; - rc = ntb_mw_get_align(ndev, PIDX, i, &mw->xlat_align, - &mw->xlat_align_size, NULL); - if (rc) - goto err1; - rc = ntb_peer_mw_get_addr(ndev, i, &mw->phys_addr, &mw->phys_size); if (rc) diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c index 759f772fa00c..427112cf101a 100644 --- a/drivers/ntb/test/ntb_perf.c +++ b/drivers/ntb/test/ntb_perf.c @@ -108,8 +108,6 @@ MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)") struct perf_mw { phys_addr_t phys_addr; resource_size_t phys_size; - resource_size_t xlat_align; - resource_size_t xlat_align_size; void __iomem *vbase; size_t xlat_size; size_t buf_size; @@ -472,13 +470,20 @@ static int perf_set_mw(struct perf_ctx *perf, resource_size_t size) { struct perf_mw *mw = &perf->mw; size_t xlat_size, buf_size; + resource_size_t xlat_align; + resource_size_t xlat_align_size; int rc; if (!size) return -EINVAL; - xlat_size = round_up(size, mw->xlat_align_size); - buf_size = round_up(size, mw->xlat_align); + rc = ntb_mw_get_align(perf->ntb, PIDX, 0, &xlat_align, + &xlat_align_size, NULL); + if (rc) + return rc; + + xlat_size = round_up(size, xlat_align_size); + buf_size = round_up(size, xlat_align); if (mw->xlat_size == xlat_size) return 0; @@ -567,11 +572,6 @@ static int perf_setup_mw(struct ntb_dev *ntb, struct perf_ctx *perf) mw = &perf->mw; - rc = ntb_mw_get_align(ntb, PIDX, 0, &mw->xlat_align, - &mw->xlat_align_size, NULL); - if (rc) - return rc; - rc = ntb_peer_mw_get_addr(ntb, 0, &mw->phys_addr, &mw->phys_size); if (rc) return rc; diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c index a69815c45ce6..91526a986caa 100644 --- a/drivers/ntb/test/ntb_tool.c +++ b/drivers/ntb/test/ntb_tool.c @@ -753,9 +753,9 @@ static ssize_t tool_peer_mw_trans_read(struct file *filep, phys_addr_t base; resource_size_t mw_size; - resource_size_t align_addr; - resource_size_t align_size; - resource_size_t max_size; + resource_size_t align_addr = 0; + resource_size_t align_size = 0; + resource_size_t max_size = 0; buf_size = min_t(size_t, size, 512);
With Switchtec hardware it's impossible to get the alignment parameters for a peer's memory window until the peer's driver has configured it's windows. Strictly speaking, the link doesn't have to be up for this, but the link being up is the only way the client can tell that the otherside has been configured. This patch converts ntb_transport and ntb_perf to use this function after the link goes up. This simplifies these clients slightly because they no longer have to store the alignment parameters. It also tweaks ntb_tool so that peer_mw_trans will print zero if it is run before the link goes up. Signed-off-by: Logan Gunthorpe <logang@deltatee.com> --- drivers/ntb/ntb_transport.c | 20 ++++++++++---------- drivers/ntb/test/ntb_perf.c | 18 +++++++++--------- drivers/ntb/test/ntb_tool.c | 6 +++--- 3 files changed, 22 insertions(+), 22 deletions(-)