* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:01 UTC (permalink / raw)
To: Doug Ledford; +Cc: Paolo Abeni, linux-rdma, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <22f61258-9bcf-0adc-f23f-79a4f1d50c6a@redhat.com>
On Tue, Oct 11, 2016 at 01:41:56PM -0400, Doug Ledford wrote:
> declare the header. The problem then became that the sg setup is such
> that we are limited to 16 4k pages for the sg array, so that header had
> to come out of the 64k maximum mtu.
Oh, that clarifies things..
Hum, so various options become:
- Use >=17 SGL entries when creating the QP. Is this possible
on common adapters?
- Use the FRWR infrastructure when necessary. Is there any chance
the majority of skbs will have at least two physically
continuous pages to make this overhead rare? Perhaps as a fall
back if many adaptors can do >=17 SGLs
- Pad the hard header out to 4k and discard the first page
when building the sgl
- Memcopy the first ~8k into a contiguous 8k region on send
- Move the pseudo header to the end so it can cross the page
barrier without needing a sgl entry. (probably impossible?)
>From Paolo
> AFAICS the max mtu is already underlying h/w dependent, how does such
> differences are currently coped by ? (I'm sorry I lack some/a lot of IB
> back-ground)
It isn't h/w dependent. In CM mode the MTU is 65520 because that is
what is hard coded into the ipoib driver. We tell everyone to use that
number. Eg see RH's docs on the subject:
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
AFAIK, today everyone just wires that number into their scripts, so we
have to mass change everything to the smaller number. That sounds
really hard, IMHO if there is any way to avoid it we should, even if
it is a little costly.
Jason
^ permalink raw reply
* Re: [PATCH rdma-core 1/5] Pull uninitialized_var into util/compiler.h
From: Jason Gunthorpe @ 2016-10-11 18:05 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161010040930.GF9282-2ukJVAZIZ/Y@public.gmane.org>
On Mon, Oct 10, 2016 at 07:09:30AM +0300, Leon Romanovsky wrote:
> > I think people would complain about the extra stores. gcc 6 will
> > eliminate them, but older compilers will not.
>
> This unintialized_var(x) adds extra store too (... x = x ...).
I have confirmed that the unintialized_var macro does not impact code
generation on the old gccs while the =0 approach does.
So no extra store.
Jason
--
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
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-11 18:10 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma, Sean Hefty, Hal Rosenstock, netdev
In-Reply-To: <20161011180119.GB17319@obsidianresearch.com>
On Tue, 2016-10-11 at 12:01 -0600, Jason Gunthorpe wrote:
> > AFAICS the max mtu is already underlying h/w dependent, how does such
> > differences are currently coped by ? (I'm sorry I lack some/a lot of IB
> > back-ground)
>
> It isn't h/w dependent. In CM mode the MTU is 65520 because that is
> what is hard coded into the ipoib driver. We tell everyone to use that
> number. Eg see RH's docs on the subject:
>
> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
>
> AFAIK, today everyone just wires that number into their scripts, so we
> have to mass change everything to the smaller number. That sounds
> really hard, IMHO if there is any way to avoid it we should, even if
> it is a little costly.
Thank you for the details!
The first s/g fragment (the head buffer) is not allocated with the page
allocator, so perhaps there is some not too difficult/costly way out of
this.
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-11 18:17 UTC (permalink / raw)
To: Paolo Abeni, Jason Gunthorpe
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty, Hal Rosenstock,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476209407.448.9.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1622 bytes --]
On 10/11/2016 2:10 PM, Paolo Abeni wrote:
> On Tue, 2016-10-11 at 12:01 -0600, Jason Gunthorpe wrote:
>>> AFAICS the max mtu is already underlying h/w dependent, how does such
>>> differences are currently coped by ? (I'm sorry I lack some/a lot of IB
>>> back-ground)
>>
>> It isn't h/w dependent. In CM mode the MTU is 65520 because that is
>> what is hard coded into the ipoib driver. We tell everyone to use that
>> number. Eg see RH's docs on the subject:
>>
>> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Networking_Guide/sec-Configuring_IPoIB.html
>>
>> AFAIK, today everyone just wires that number into their scripts, so we
>> have to mass change everything to the smaller number.
Well, not exactly. Even if we put 65520 into the scripts, the kernel
will silently drop it down to 65504. It actually won't require anyone
change anything, they just won't get the full value. I experimented
with this in the past for other reasons and an overly large MTU setting
just resulted in the max MTU. I don't know if that's changed, but if it
still works that way, this is much less of an issue than it might
otherwise be.
>> That sounds
>> really hard, IMHO if there is any way to avoid it we should, even if
>> it is a little costly.
>
> Thank you for the details!
>
> The first s/g fragment (the head buffer) is not allocated with the page
> allocator, so perhaps there is some not too difficult/costly way out of
> this.
>
>
>
>
>
>
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:27 UTC (permalink / raw)
To: Paolo Abeni
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476209407.448.9.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 08:10:07PM +0200, Paolo Abeni wrote:
> The first s/g fragment (the head buffer) is not allocated with the page
> allocator, so perhaps there is some not too difficult/costly way out of
> this.
Keep in mind, there is nothing magic about the 16 SGL limit, other
than we know all hardware supports it. That can be bumped up and most
hardware will support a higher value.
We'd just have to figure out if any hardware breaks, Mellanox and Intel
should be able to respond to that question.
Jason
--
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
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Jason Gunthorpe @ 2016-10-11 18:30 UTC (permalink / raw)
To: Doug Ledford
Cc: Paolo Abeni, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <073e0007-a43b-134d-ba7e-b290304be585-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Oct 11, 2016 at 02:17:51PM -0400, Doug Ledford wrote:
> Well, not exactly. Even if we put 65520 into the scripts, the kernel
> will silently drop it down to 65504. It actually won't require anyone
> change anything, they just won't get the full value. I experimented
> with this in the past for other reasons and an overly large MTU setting
> just resulted in the max MTU. I don't know if that's changed, but if it
> still works that way, this is much less of an issue than it might
> otherwise be.
So it is just docs and relying on PMTU? That is not as bad..
Still would be nice to avoid if at all possible..
Jason
--
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
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-11 18:50 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Paolo Abeni, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sean Hefty,
Hal Rosenstock, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011183019.GC20253-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]
On 10/11/2016 2:30 PM, Jason Gunthorpe wrote:
> On Tue, Oct 11, 2016 at 02:17:51PM -0400, Doug Ledford wrote:
>
>> Well, not exactly. Even if we put 65520 into the scripts, the kernel
>> will silently drop it down to 65504. It actually won't require anyone
>> change anything, they just won't get the full value. I experimented
>> with this in the past for other reasons and an overly large MTU setting
>> just resulted in the max MTU. I don't know if that's changed, but if it
>> still works that way, this is much less of an issue than it might
>> otherwise be.
>
> So it is just docs and relying on PMTU? That is not as bad..
>
> Still would be nice to avoid if at all possible..
I agree, but we have a test getting ready to commence. We'll know
shortly how much the reduced MTU effects things because they aren't
going to alter any of their setup, just put the new kernel in place, and
see what happens.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 03/11] IB/sa: Modify SM Address handle to program GRH when using large lids
From: Chandramouli, Dasaratharaman @ 2016-10-11 19:19 UTC (permalink / raw)
To: Jason Gunthorpe, ira.weiny-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Don Hiatt
In-Reply-To: <20160923183217.GD13920-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
The reason for doing this in the kernel was that we wanted a more
uniform approach in specifying extended LIDs across the user and the
kernel space. This also meant we could leave the ah_attr.dlid field in
the kernel to be 16 bits.
We will extend ah_attr.dlid to 32 bits in the kernel per your
suggestion. This should make specifying extended LID information in the
GRH of the address handle unnecessary.
Thanks,
Dasa
On 9/23/2016 11:32 AM, Jason Gunthorpe wrote:
> On Fri, Sep 23, 2016 at 01:44:26PM -0400, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
>> From: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>> When either the port lid or the sm lid is above the ib unicast lid
>> space, the SMI creates an address handle with the revelant GRH
>> information.
>
> Woah, I though you were only using that horrible GID hack for the
> uABI.
>
> Don't do such crazy things in the kernel. Fix the internal kernel to
> support your larger lid.
>
> Jason
>
--
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
* Re: build failure on linus' latest master branch
From: Leon Romanovsky @ 2016-10-11 19:36 UTC (permalink / raw)
To: Steve Wise; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <004701d223cc$0772cbb0$16586310$@opengridcomputing.com>
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]
On Tue, Oct 11, 2016 at 09:30:35AM -0500, Steve Wise wrote:
> > > Is there a fix for this?
> >
> > We investigated a lot this failure and it appears on specific and very
> > old GCC 4.4.
> > http://www.spinics.net/lists/linux-rdma/msg39135.html
> > http://marc.info/?t=147614305200001&r=1&w=2
> >
> > And the fix is http://marc.info/?l=linux-netdev&m=147615976207362&w=2
> >
>
> Thanks. So was it ever merged?
Not yet, I hope that Dave or Doug will forward that patch to Linus
before -rc1 is released.
>
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-core 1/5] Pull uninitialized_var into util/compiler.h
From: Leon Romanovsky @ 2016-10-11 19:46 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011180517.GA17866-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On Tue, Oct 11, 2016 at 12:05:17PM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 10, 2016 at 07:09:30AM +0300, Leon Romanovsky wrote:
>
> > > I think people would complain about the extra stores. gcc 6 will
> > > eliminate them, but older compilers will not.
> >
> > This unintialized_var(x) adds extra store too (... x = x ...).
>
> I have confirmed that the unintialized_var macro does not impact code
> generation on the old gccs while the =0 approach does.
Ohh, thanks.
But I'm still left under impression of this article [1] that using such
macro is a bad thing and we are "punishing" all users of modern compilers.
[1] https://lwn.net/Articles/529954/
>
> So no extra store.
>
> Jason
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH rdma-core 1/5] Pull uninitialized_var into util/compiler.h
From: Jason Gunthorpe @ 2016-10-11 20:06 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011194633.GP9282-2ukJVAZIZ/Y@public.gmane.org>
On Tue, Oct 11, 2016 at 10:46:33PM +0300, Leon Romanovsky wrote:
> But I'm still left under impression of this article [1] that using such
> macro is a bad thing and we are "punishing" all users of modern compilers.
I agree with the article, it is a bad idea. This is why my version is
disabling the macro entirely if gcc 6 or clang is used - aka the
compilers that run in Travis.
So, new code must not introduce control flow that is more complex than
gcc 6 can understand, and the macro is used only for gcc 4.x and 5.x
compatability to provide warning free compile on popular distros
without a performance hit.
Jason
--
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
* Re: build failure on linus' latest master branch
From: Doug Ledford @ 2016-10-11 20:47 UTC (permalink / raw)
To: Leon Romanovsky, Steve Wise; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161011193633.GN9282-2ukJVAZIZ/Y@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 715 bytes --]
On 10/11/2016 3:36 PM, Leon Romanovsky wrote:
> On Tue, Oct 11, 2016 at 09:30:35AM -0500, Steve Wise wrote:
>>>> Is there a fix for this?
>>>
>>> We investigated a lot this failure and it appears on specific and very
>>> old GCC 4.4.
>>> http://www.spinics.net/lists/linux-rdma/msg39135.html
>>> http://marc.info/?t=147614305200001&r=1&w=2
>>>
>>> And the fix is http://marc.info/?l=linux-netdev&m=147615976207362&w=2
>>>
>>
>> Thanks. So was it ever merged?
>
> Not yet, I hope that Dave or Doug will forward that patch to Linus
> before -rc1 is released.
That patch isn't even on my radar.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH v2 7/7] [RFC] nvme: Fix a race condition
From: Bart Van Assche @ 2016-10-12 0:41 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, James Bottomley, Martin K. Petersen, Mike Snitzer,
Doug Ledford, Keith Busch, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-nvme@lists.infradead.org
In-Reply-To: <20161011164601.GC18822@lst.de>
On 10/11/16 09:46, Christoph Hellwig wrote:
> On Wed, Sep 28, 2016 at 05:01:45PM -0700, Bart Van Assche wrote:
>> Avoid that nvme_queue_rq() is still running when nvme_stop_queues()
>> returns. Untested.
>>
>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
>> Cc: Keith Busch <keith.busch@intel.com>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Cc: Sagi Grimberg <sagi@grimberg.me>
>> ---
>> drivers/nvme/host/core.c | 20 ++++++++++++--------
>> 1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index d791fba..98f1f29 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -201,13 +201,9 @@ fail:
>>
>> void nvme_requeue_req(struct request *req)
>> {
>> - unsigned long flags;
>> -
>> blk_mq_requeue_request(req);
>> - spin_lock_irqsave(req->q->queue_lock, flags);
>> - if (!blk_mq_queue_stopped(req->q))
>> - blk_mq_kick_requeue_list(req->q);
>> - spin_unlock_irqrestore(req->q->queue_lock, flags);
>> + WARN_ON_ONCE(blk_mq_queue_stopped(req->q));
>> + blk_mq_kick_requeue_list(req->q);
>> }
>> EXPORT_SYMBOL_GPL(nvme_requeue_req);
>
> Can we just add a 'bool kick' argument to blk_mq_requeue_request and
> move all this handling to the core?
Hello Christoph,
That sounds like a good idea to me. Thanks also for the other review
comments you posted on this patch series. I will rework patch 6/7 such
that the code for waiting is moved into the SCSI core.
Bart.
^ permalink raw reply
* [bug report] IB/hfi1: Prevent NULL pointer deferences in caching code
From: Dan Carpenter @ 2016-10-12 6:06 UTC (permalink / raw)
To: mitko.haralanov-ral2JQCrhuEAvxtiuMwx3w; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello Mitko Haralanov,
The patch f19bd643dbde: "IB/hfi1: Prevent NULL pointer deferences in
caching code" from Apr 12, 2016, leads to the following static
checker warning:
drivers/infiniband/hw/hfi1/user_sdma.c:1147 pin_vector_pages()
warn: 'rb_node' isn't an ERR_PTR
drivers/infiniband/hw/hfi1/user_sdma.c
1135 static int pin_vector_pages(struct user_sdma_request *req,
1136 struct user_sdma_iovec *iovec)
1137 {
1138 int ret = 0, pinned, npages, cleared;
1139 struct page **pages;
1140 struct hfi1_user_sdma_pkt_q *pq = req->pq;
1141 struct sdma_mmu_node *node = NULL;
1142 struct mmu_rb_node *rb_node;
1143
1144 rb_node = hfi1_mmu_rb_extract(pq->handler,
1145 (unsigned long)iovec->iov.iov_base,
1146 iovec->iov.iov_len);
1147 if (rb_node && !IS_ERR(rb_node))
^^^^^^^^^^^^^^^
hfi1_mmu_rb_extract() never returns error pointers. Plz delete.
1148 node = container_of(rb_node, struct sdma_mmu_node, rb);
1149 else
1150 rb_node = NULL;
1151
1152 if (!node) {
1153 node = kzalloc(sizeof(*node), GFP_KERNEL);
1154 if (!node)
1155 return -ENOMEM;
1156
1157 node->rb.addr = (unsigned long)iovec->iov.iov_base;
1158 node->pq = pq;
1159 atomic_set(&node->refcount, 0);
1160 }
regards,
dan carpenter
--
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
* Re: build failure on linus' latest master branch
From: Leon Romanovsky @ 2016-10-12 8:42 UTC (permalink / raw)
To: Doug Ledford; +Cc: Steve Wise, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <c2dffb44-d600-40d6-ec5d-09ef917da877-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Tue, Oct 11, 2016 at 04:47:53PM -0400, Doug Ledford wrote:
> On 10/11/2016 3:36 PM, Leon Romanovsky wrote:
> > On Tue, Oct 11, 2016 at 09:30:35AM -0500, Steve Wise wrote:
> >>>> Is there a fix for this?
> >>>
> >>> We investigated a lot this failure and it appears on specific and very
> >>> old GCC 4.4.
> >>> http://www.spinics.net/lists/linux-rdma/msg39135.html
> >>> http://marc.info/?t=147614305200001&r=1&w=2
> >>>
> >>> And the fix is http://marc.info/?l=linux-netdev&m=147615976207362&w=2
> >>>
> >>
> >> Thanks. So was it ever merged?
> >
> > Not yet, I hope that Dave or Doug will forward that patch to Linus
> > before -rc1 is released.
>
> That patch isn't even on my radar.
I had in mind to wait till v2 will be sent before asking you and/or Dave
forward it to Linus.
http://marc.info/?l=linux-netdev&m=147623786131380&w=2
Thanks
>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> GPG Key ID: 0E572FDD
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V2 net-next] net/mlx5: Add MLX5_ARRAY_SET64 to fix BUILD_BUG_ON
From: Leon Romanovsky @ 2016-10-12 8:50 UTC (permalink / raw)
To: David S. Miller, Doug Ledford
Cc: Saeed Mahameed, netdev-u79uwXL29TY76Z2rM5mHXA, Tom Herbert,
David Laight, kernel-team-b10kYP2dOMg, RDMA mailing list,
Steve Wise
In-Reply-To: <1476237430-11756-1-git-send-email-saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4097 bytes --]
On Wed, Oct 12, 2016 at 04:57:10AM +0300, Saeed Mahameed wrote:
> From: Tom Herbert <tom-BjP2VixgY4xUbtYUoyoikg@public.gmane.org>
>
> I am hitting this in mlx5:
>
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function
> reclaim_pages_cmd.clone.0:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:346: error: call
> to __compiletime_assert_346 declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_out, pas[i]) % 64
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c: In function give_pages:
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:291: error: call
> to __compiletime_assert_291 declared with attribute error:
> BUILD_BUG_ON failed: __mlx5_bit_off(manage_pages_in, pas[i]) % 64
>
> Problem is that this is doing a BUILD_BUG_ON on a non-constant
> expression because of trying to take offset of pas[i] in the
> structure.
>
> Fix is to create MLX5_ARRAY_SET64 that takes an additional argument
> that is the field index to separate between BUILD_BUG_ON on the array
> constant field and the indexed field to assign the value to.
> There are two callers of MLX5_SET64 that are trying to get a variable
> offset, change those to call MLX5_ARRAY_SET64 passing 'pas' and 'i'
> as the arguments to use in the offset check and the indexed value
> assignment.
>
> Fixes: a533ed5e179cd ("net/mlx5: Pages management commands via mlx5 ifc")
> Signed-off-by: Tom Herbert <tom-BjP2VixgY4xUbtYUoyoikg@public.gmane.org>
> Signed-off-by: Saeed Mahameed <saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>
> Hi Dave,
>
> I hope this version of this patch will make it to -rc1, I made
> some changes to the original version Tom submitted. Following David Laight
> suggestion to separate the array index from the constant array field to have
> a more natural API.
>
> Thanks,
> Saeed.
>
>
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c | 4 ++--
> include/linux/mlx5/device.h | 13 +++++++++++--
> 2 files changed, 13 insertions(+), 4 deletions(-)
Hi Dave and Doug,
Any chances that this fix will be forwarded to Linus in merge window?
I don't know if you (Dave) had in mind to send additional pull request to
Linus in this merge window, but Doug is definitely planning, and it will
be awesome if it includes this fix in it.
Thanks
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> index d458515..cc4fd61 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c
> @@ -287,7 +287,7 @@ retry:
>
> goto retry;
> }
> - MLX5_SET64(manage_pages_in, in, pas[i], addr);
> + MLX5_ARRAY_SET64(manage_pages_in, in, pas, i, addr);
> }
>
> MLX5_SET(manage_pages_in, in, opcode, MLX5_CMD_OP_MANAGE_PAGES);
> @@ -344,7 +344,7 @@ static int reclaim_pages_cmd(struct mlx5_core_dev *dev,
> if (fwp->func_id != func_id)
> continue;
>
> - MLX5_SET64(manage_pages_out, out, pas[i], fwp->addr);
> + MLX5_ARRAY_SET64(manage_pages_out, out, pas, i, fwp->addr);
> i++;
> }
>
> diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
> index 77c1417..5827614 100644
> --- a/include/linux/mlx5/device.h
> +++ b/include/linux/mlx5/device.h
> @@ -92,12 +92,21 @@ __mlx5_mask(typ, fld))
> ___t; \
> })
>
> -#define MLX5_SET64(typ, p, fld, v) do { \
> +#define __MLX5_SET64(typ, p, fld, v) do { \
> BUILD_BUG_ON(__mlx5_bit_sz(typ, fld) != 64); \
> - BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> *((__be64 *)(p) + __mlx5_64_off(typ, fld)) = cpu_to_be64(v); \
> } while (0)
>
> +#define MLX5_SET64(typ, p, fld, v) do { \
> + BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> + __MLX5_SET64(typ, p, fld, v); \
> +} while (0)
> +
> +#define MLX5_ARRAY_SET64(typ, p, fld, idx, v) do { \
> + BUILD_BUG_ON(__mlx5_bit_off(typ, fld) % 64); \
> + __MLX5_SET64(typ, p, fld[idx], v); \
> +} while (0)
> +
> #define MLX5_GET64(typ, p, fld) be64_to_cpu(*((__be64 *)(p) + __mlx5_64_off(typ, fld)))
>
> #define MLX5_GET64_PR(typ, p, fld) ({ \
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] add a build.sh helper to allow for a one-stop build
From: Leon Romanovsky @ 2016-10-12 9:30 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Bart Van Assche, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161010164758.GB1796-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]
On Mon, Oct 10, 2016 at 10:47:58AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 10, 2016 at 09:17:11AM -0700, Bart Van Assche wrote:
> > On 10/10/16 08:59, Christoph Hellwig wrote:
> > >On Mon, Oct 10, 2016 at 08:56:33AM -0700, Bart Van Assche wrote:
> > >>Have you considered to make this script stop immediately if one of the
> > >>steps fails, e.g. as follows?
> > >
> > >How about just ading a
> > >
> > >set -e
> > >
> > >to the beginning to get this automatically?
> >
> > That would also work. But if there is a space anywhere in the path in which
> > the rdma-core repository exists, mkdir $BUILDDIR will have to be changed
> > into mkdir "$BUILDDIR".
>
> Is this OK?
>
> From 6362ec4c1ddec0f6b2a1bf052cdfde63cbf73d90 Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Mon, 10 Oct 2016 10:46:47 -0600
> Subject: [PATCH] Add a build.sh helper to allow for a one-stop build
>
> This allows for a quick build instead of typing the whole mkdir, cd,
> cmake and ninja sequence.
>
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
> README.md | 17 +----------------
> build.sh | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 16 deletions(-)
> create mode 100755 build.sh
>
> diff --git a/README.md b/README.md
> index 98ec5a7d695f..66aee3f49f00 100644
> --- a/README.md
> +++ b/README.md
> @@ -36,10 +36,7 @@ Additional service daemons are provided for:
> This project uses a cmake based build system. Quick start:
>
> ```sh
> -$ mkdir build
> -$ cd build
> -$ cmake -GNinja ..
> -$ ninja
> +$ bash build.sh
> ```
>
> *build/bin* will contain the sample programs and *build/lib* will contain the
> @@ -76,16 +73,6 @@ Install required packages:
> $ yum install cmake gcc libnl3-devel make pkgconfig valgrind-devel
> ```
>
> -For end users, the package can be built using GNU Make and the old cmake
> -included with the distro:
> -
> -```sh
> -$ mkdir build
> -$ cd build
> -$ cmake ..
> -$ make
> -```
> -
> Developers are suggested to install more modern tooling for the best experience.
>
> ```sh
> @@ -96,8 +83,6 @@ $ unzip ninja-linux.zip
> $ install -m755 ninja /usr/local/bin/ninja
> ```
>
> -Use the 'cmake3' program in place of `cmake` in the above instructions.
> -
> # Reporting bugs
>
> Bugs should be reported to the <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> mailing list
> diff --git a/build.sh b/build.sh
> new file mode 100755
> index 000000000000..76205e6af921
> --- /dev/null
> +++ b/build.sh
> @@ -0,0 +1,33 @@
> +#!/bin/bash
> +set -e
> +
> +SRCDIR=`dirname $0`
> +BUILDDIR="$SRCDIR/build"
> +
> +if [ ! -d "$BUILDDIR" ]; then
> + mkdir "$BUILDDIR"
> +fi
What do you think about following code, instead of 3 lines above?
+ mkdir -p "$BUILDDIR"
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* rdma-core release?
From: Christoph Hellwig @ 2016-10-12 10:51 UTC (permalink / raw)
To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Are we planning on having a rdma-core release now that 4.8 is out
and we want a matching userspace release?
--
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
* Re: [PATCH] add a build.sh helper to allow for a one-stop build
From: Christoph Hellwig @ 2016-10-12 10:52 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Jason Gunthorpe, Bart Van Assche, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161012093015.GS9282-2ukJVAZIZ/Y@public.gmane.org>
[full quote deleted, sigh..]
> What do you think about following code, instead of 3 lines above?
>
> + mkdir -p "$BUILDDIR"
Fine with me. Feel free to send an incremental patch.
--
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
* Re: rdma-core release?
From: Leon Romanovsky @ 2016-10-12 11:43 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-rdma
In-Reply-To: <20161012105103.GA30534-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
On Wed, Oct 12, 2016 at 1:51 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> Are we planning on having a rdma-core release now that 4.8 is out
> and we want a matching userspace release?
As far as I remember, the initial plan was to use this library
immediately, but first release was planned to be at the 4.9
announcement to ensure that it runs smoothly.
> --
> 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
--
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
* Re: [PATCH] add a build.sh helper to allow for a one-stop build
From: Leon Romanovsky @ 2016-10-12 12:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, Bart Van Assche,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161012105208.GA9491-jcswGhMUV9g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 357 bytes --]
On Wed, Oct 12, 2016 at 12:52:08PM +0200, Christoph Hellwig wrote:
> [full quote deleted, sigh..]
>
> > What do you think about following code, instead of 3 lines above?
> >
> > + mkdir -p "$BUILDDIR"
>
> Fine with me. Feel free to send an incremental patch.
https://github.com/linux-rdma/rdma-core/pull/17
I'll apply it once travis will finish.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] add a build.sh helper to allow for a one-stop build
From: Leon Romanovsky @ 2016-10-12 12:22 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jason Gunthorpe, Bart Van Assche,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161012121845.GT9282-2ukJVAZIZ/Y@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 466 bytes --]
On Wed, Oct 12, 2016 at 03:18:45PM +0300, Leon Romanovsky wrote:
> On Wed, Oct 12, 2016 at 12:52:08PM +0200, Christoph Hellwig wrote:
> > [full quote deleted, sigh..]
> >
> > > What do you think about following code, instead of 3 lines above?
> > >
> > > + mkdir -p "$BUILDDIR"
> >
> > Fine with me. Feel free to send an incremental patch.
>
> https://github.com/linux-rdma/rdma-core/pull/17
>
> I'll apply it once travis will finish.
Done, applied.
>
> Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: rdma-core release?
From: Doug Ledford @ 2016-10-12 13:00 UTC (permalink / raw)
To: Leon Romanovsky, Christoph Hellwig; +Cc: linux-rdma
In-Reply-To: <CALq1K=JEkvN_4tZL9D53c6-EH9E4efay8rmoyBEyB-7Sxr6H-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 566 bytes --]
On 10/12/2016 7:43 AM, Leon Romanovsky wrote:
> On Wed, Oct 12, 2016 at 1:51 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
>> Are we planning on having a rdma-core release now that 4.8 is out
>> and we want a matching userspace release?
>
> As far as I remember, the initial plan was to use this library
> immediately, but first release was planned to be at the 4.9
> announcement to ensure that it runs smoothly.
Correct.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [bug report] IB/hfi1: Prevent NULL pointer deferences in caching code
From: Dalessandro, Dennis @ 2016-10-12 13:43 UTC (permalink / raw)
To: Haralanov, Mitko,
dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161012060402.GC12841@mwanda>
Thanks Dan, we'll take care of it.
-Denny
On Wed, 2016-10-12 at 09:06 +0300, Dan Carpenter wrote:
> Hello Mitko Haralanov,
>
> The patch f19bd643dbde: "IB/hfi1: Prevent NULL pointer deferences in
> caching code" from Apr 12, 2016, leads to the following static
> checker warning:
>
> drivers/infiniband/hw/hfi1/user_sdma.c:1147 pin_vector_pages()
> warn: 'rb_node' isn't an ERR_PTR
>
> drivers/infiniband/hw/hfi1/user_sdma.c
> 1135 static int pin_vector_pages(struct user_sdma_request *req,
> 1136 struct user_sdma_iovec *iovec)
> 1137 {
> 1138 int ret = 0, pinned, npages, cleared;
> 1139 struct page **pages;
> 1140 struct hfi1_user_sdma_pkt_q *pq = req->pq;
> 1141 struct sdma_mmu_node *node = NULL;
> 1142 struct mmu_rb_node *rb_node;
> 1143
> 1144 rb_node = hfi1_mmu_rb_extract(pq->handler,
> 1145 (unsigned long)iovec-
> >iov.iov_base,
> 1146 iovec->iov.iov_len);
> 1147 if (rb_node && !IS_ERR(rb_node))
> ^^^^^^^^^^^^^^^
>
> hfi1_mmu_rb_extract() never returns error pointers. Plz delete.
>
> 1148 node = container_of(rb_node, struct
> sdma_mmu_node, rb);
> 1149 else
> 1150 rb_node = NULL;
> 1151
> 1152 if (!node) {
> 1153 node = kzalloc(sizeof(*node), GFP_KERNEL);
> 1154 if (!node)
> 1155 return -ENOMEM;
> 1156
> 1157 node->rb.addr = (unsigned long)iovec-
> >iov.iov_base;
> 1158 node->pq = pq;
> 1159 atomic_set(&node->refcount, 0);
> 1160 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* false checkpatch finding?
From: Marciniszyn, Mike @ 2016-10-12 14:04 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
<4.8 tree>/scripts/checkpatch.pl -F foo.h
WARNING: Missing a blank line after declarations
#3: FILE: foo.h:3:
+ unsigned long f1;
+ volatile __le64 f2.
WARNING: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
#3: FILE: foo.h:3:
+ volatile __le64 f2.
total: 0 errors, 2 warnings, 4 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
foo.h has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Adding a gratuitous blank line after f1 silences the bogus warning.
The volatile warning is ok because this is a hardware written field.
Snip the test file from below.
Mike
<snip foo.h>
struct foo {
unsigned long f1;
volatile __le64 f2.
};
</snip foo.h>
--
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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox