* update rdma-plumbing Documentation
From: Christoph Hellwig @ 2016-09-19 15:56 UTC (permalink / raw)
To: jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Get rid of the per-subproject README files and instead improve the top-level
README.md as well as adding a few new files for more details to Documentation/
--
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: move unsafe global rkey handling to the RDMA core V2
From: Christoph Hellwig @ 2016-09-19 15:54 UTC (permalink / raw)
To: Christoph Hellwig
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1473072981-2035-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>
Doug, any comments on this series? I'd really like to get it in for
this merge window.
--
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] ibacm: Handle EP expiration time
From: Hefty, Sean @ 2016-09-19 15:51 UTC (permalink / raw)
To: Yuval Shaia; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160919072504.GA3571-Hxa29pjIrETlQW142y8m19+IiqhCXseY@public.gmane.org>
> > > static struct acmp_dest *
> > > acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const
> uint8_t
> > > *addr)
> > > {
> > > @@ -366,6 +382,15 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t
> > > addr_type, const uint8_t *addr)
> > > acm_log(2, "%s\n", log_data);
> > > lock_acquire(&ep->lock);
> > > dest = acmp_get_dest(ep, addr_type, addr);
> > > + if (dest && is_dest_ready(dest)) {
> >
> > I think it would be clearer to just perform the state check here and
> either avoid the timeout check or merge it with the if statement below.
>
> As stated above, i cannot ignore the timeout so will merge it here.
>
> >
> > > + acm_log(2, "Record valid for the next %ld minute(s)\n",
> > > + dest->addr_timeout - time_stamp_min());
> > > + if (time_stamp_min() >= dest->addr_timeout) {
We also check addr_timeout here. (Sorry, it's been a long while since I've looked at this code.) My question regarding timeouts are around why do we need two checks?
--
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 rdma-next V1 3/8] IB/cxgb4: Move user vendor structures
From: Steve Wise @ 2016-09-19 15:30 UTC (permalink / raw)
To: 'Leon Romanovsky', dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474195823-10429-4-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> This patch moves cxgb4 vendor's specific structures to
> common UAPI folder which will be visible to all consumers.
>
> These structures are used by user-space library driver
> (libcxgb4) and currently manually copied to that library.
>
> This move will allow cross-compile against these files and
> simplify introduction of vendor specific data.
>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Looks good.
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
--
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 rdma-next V1 2/8] IB/cxgb3: Move user vendor structures
From: Steve Wise @ 2016-09-19 15:30 UTC (permalink / raw)
To: 'Leon Romanovsky', dledford-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474195823-10429-3-git-send-email-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> This patch moves cxgb3 vendor's specific structures to
> common UAPI folder which will be visible to all consumers.
>
> These structures are used by user-space library driver
> (libcxgb3) and currently manually copied to that library.
>
> This move will allow cross-compile against these files and
> simplify introduction of vendor specific data.
>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Looks good.
Reviewed-by: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
--
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 09/28] mlx5: Fix gcc 6.4 uninitialized variable warning
From: Yishai Hadas @ 2016-09-19 15:02 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Devesh Sharma,
Hal Rosenstock, Mike Marciniszyn, Moni Shoua, Sean Hefty,
Steve Wise, Tatyana Nikolova, Vladimir Sokolovsky, Yishai Hadas
In-Reply-To: <20160915162136.GA26111-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On 9/15/2016 7:21 PM, Jason Gunthorpe wrote:
> On Thu, Sep 15, 2016 at 05:26:33PM +0300, Yishai Hadas wrote:
>
>> This 'if' can be omitted and its body can be embedded above for both
>> lazy/non-lazy modes.
> Sure
>> Will handle that in our side and put into our regression system, once the
>> patch will be approved will take it into my formal GIT so that you can take
>> it from there.
>
> Great on both patches
Made the above change in addition to some changes in the commit logs and
applied both.
The other patches for libmlx4/5 were applied as well, thanks.
--
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 0/2] cxgb4 FR_NSMR_TPTE_WR support
From: David Miller @ 2016-09-19 14:29 UTC (permalink / raw)
To: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <007f01d21280$7d1556b0$77400410$@opengridcomputing.com>
From: "Steve Wise" <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
Date: Mon, 19 Sep 2016 09:17:01 -0500
>
>>
>> From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
>> Date: Fri, 16 Sep 2016 07:54:55 -0700
>>
>> > This series enables a new work request to optimize small REG_MR
>> > operations. This is intended for 4.9. If everyone agrees, I suggest
>> > Doug take both the cxgb4 and iw_cxgb4 patches through his tree.
>>
>> I'm assuming this mean that I do _not_ apply these to my tree.
>
> Yes, if you're ok with that.
I am.
--
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 0/2] cxgb4 FR_NSMR_TPTE_WR support
From: Steve Wise @ 2016-09-19 14:17 UTC (permalink / raw)
To: 'David Miller'
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160919.011633.1357649774235825501.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>
> From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
> Date: Fri, 16 Sep 2016 07:54:55 -0700
>
> > This series enables a new work request to optimize small REG_MR
> > operations. This is intended for 4.9. If everyone agrees, I suggest
> > Doug take both the cxgb4 and iw_cxgb4 patches through his tree.
>
> I'm assuming this mean that I do _not_ apply these to my tree.
Yes, if you're ok with that.
Steve.
--
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: VERBOSE flags
From: Dalessandro, Dennis @ 2016-09-19 13:34 UTC (permalink / raw)
To: leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <20160919132518.GH3273-2ukJVAZIZ/Y@public.gmane.org>
On Mon, 2016-09-19 at 16:25 +0300, Leon Romanovsky wrote:
> On Mon, Sep 19, 2016 at 12:29:43PM +0000, Dalessandro, Dennis wrote:
> > On Mon, 2016-09-19 at 09:15 +0300, Leon Romanovsky wrote:
> > > The function pointers will cause us to create two similar
> > > functions
> > > (debug/regular) for every important function. It will leave to
> > > code
> > > duplication and/or introducing of new code to fill additional
> > > fields
> > > in returns to debug functions, so they will be able to print it.
> >
> > I do sort of like Doug's idea. It means you can easily instrument
> > the
> > heck out of a routine and have it always available. It provides
> > more
> > flexibility in what you can do in a debug version.
> >
> > You do make a valid point though about duplicating code. Perhaps
> > that
> > can be alleviated to some extent just by refactoring and adding
> > inlines, etc.
>
> At the end, we will invent tracepoints :)
>
> >
> > > I wanted to propose to use tracepoints to provide debug
> > > information
> > > in
> > > data paths. The tracepoints infrastructure uses hooks in the code
> > > with
> > > minimal impact on performance.
> >
> > This is so far the path we have chosen for hfi1. We have a number
> > of
> > tracepoints in various parts of the code. Again it's available on
> > demand without special kernels or boot/driver options.
>
> CONFIG_SDMA_VERBOSITY ????
Yes we do have that, which illustrates that trace points can not do
everything. I had hoped to deprecate that at some point. Regardless,
sometimes you want additional code to be added or to do something
slightly different. You can't do that with a tracepoint.
You can see we use tracepoints pretty extensively and that is our main
vehicle for debugging. Here is where all the definitions are:
$ ls trace_*
trace_ctxts.h trace_dbg.h trace_ibhdrs.h trace_misc.h trace_rc.h t
race_rx.h trace_tx.h
-Denny
> >
> > > > Then we can drop the CONFIG_*_DEBUG options out of all of the
> > > > drivers. That would be my preference. I don't want any
> > > > compile
> > > > time
> > > > options because you never will be running a debug kernel when
> > > > you
> > > > need
> > > > it. Per device, run time selectable so that you get 0 overhead
> > > > in
> > > > the
> > > > normal case is what I would like to see.
> > >
> > > Great,
> > > We are aligned on this point, so do I. I see no reason in special
> > > compiled
> > > kernel or global module parameter to print debug prints for whole
> > > driver.
> >
> > Same here.
> >
> > -Denny
^ permalink raw reply
* Re: [PATCH] IB/rxe: avoid putting a large struct rxe_qp on stack
From: Leon Romanovsky @ 2016-09-19 13:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Moni Shoua,
Yonatan Cohen, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160919115826.553858-1-arnd-r2nGTMty4D4@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 896 bytes --]
On Mon, Sep 19, 2016 at 01:57:26PM +0200, Arnd Bergmann wrote:
> A race condition fix added an rxe_qp structure to the stack in order
> to be able to perform rollback in rxe_requester(), but the structure
> is large enough to trigger the warning for possible stack overflow:
>
> drivers/infiniband/sw/rxe/rxe_req.c: In function 'rxe_requester':
> drivers/infiniband/sw/rxe/rxe_req.c:757:1: error: the frame size of 2064 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
>
> This changes the rollback function to only save the psn inside
> the qp, which is the only field we access in the rollback_qp
> anyway.
>
> Fixes: 3050b9985024 ("IB/rxe: Fix race condition between requester and completer")
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Thanks Arnd,
It is much cleaner approach.
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: VERBOSE flags
From: Leon Romanovsky @ 2016-09-19 13:25 UTC (permalink / raw)
To: Dalessandro, Dennis
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1474288178.6520.12.camel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]
On Mon, Sep 19, 2016 at 12:29:43PM +0000, Dalessandro, Dennis wrote:
> On Mon, 2016-09-19 at 09:15 +0300, Leon Romanovsky wrote:
> > The function pointers will cause us to create two similar functions
> > (debug/regular) for every important function. It will leave to code
> > duplication and/or introducing of new code to fill additional fields
> > in returns to debug functions, so they will be able to print it.
>
> I do sort of like Doug's idea. It means you can easily instrument the
> heck out of a routine and have it always available. It provides more
> flexibility in what you can do in a debug version.
>
> You do make a valid point though about duplicating code. Perhaps that
> can be alleviated to some extent just by refactoring and adding
> inlines, etc.
At the end, we will invent tracepoints :)
>
> > I wanted to propose to use tracepoints to provide debug information
> > in
> > data paths. The tracepoints infrastructure uses hooks in the code
> > with
> > minimal impact on performance.
>
> This is so far the path we have chosen for hfi1. We have a number of
> tracepoints in various parts of the code. Again it's available on
> demand without special kernels or boot/driver options.
CONFIG_SDMA_VERBOSITY ????
>
> > > Then we can drop the CONFIG_*_DEBUG options out of all of the
> > > drivers. That would be my preference. I don't want any compile
> > > time
> > > options because you never will be running a debug kernel when you
> > > need
> > > it. Per device, run time selectable so that you get 0 overhead in
> > > the
> > > normal case is what I would like to see.
> >
> > Great,
> > We are aligned on this point, so do I. I see no reason in special
> > compiled
> > kernel or global module parameter to print debug prints for whole
> > driver.
>
> Same here.
>
> -Denny
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v1] ibacm: Handle EP expiration time
From: Yuval Shaia @ 2016-09-19 13:21 UTC (permalink / raw)
To: Hal Rosenstock
Cc: sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <387b0fc4-949f-b8e2-6ac3-136e5ab2e127-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On Mon, Sep 19, 2016 at 07:22:54AM -0400, Hal Rosenstock wrote:
> > +
> > static struct acmp_dest *
> > acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
> > {
> > @@ -366,6 +376,16 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
> > acm_log(2, "%s\n", log_data);
> > lock_acquire(&ep->lock);
> > dest = acmp_get_dest(ep, addr_type, addr);
> > + if (dest && dest->state == ACMP_READY &&
> > + dest->addr_timeout != 0xFFFFFFFFFFFFFFFF) {
>
> Nit related to the timeout check:
> In acmp.c, (uint64_t)~0ULL is used in other places for this. Should this
> addr_timeout check be changed to some infinite or no timeout define
> along with the other occurrences for consistency and better code
> readability ?
Will do.
>
> -- Hal
>
> > + acm_log(2, "Record valid for the next %ld minute(s)\n",
> > + dest->addr_timeout - time_stamp_min());
> > + if (time_stamp_min() >= dest->addr_timeout) {
> > + acm_log(2, "Record expired\n");
> > + acmp_remove_dest(ep, dest);
> > + dest = NULL;
> > + }
--
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: [PATCHv12 1/3] rdmacg: Added rdma cgroup controller
From: Dalessandro, Dennis @ 2016-09-19 13:10 UTC (permalink / raw)
To: pandit.parav-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
corbet-T1hC0tSOHrs@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hch-jcswGhMUV9g@public.gmane.org,
linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org, Hefty, Sean,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org,
tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
liranl-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
"jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org" <jg>
In-Reply-To: <CAG53R5X4stfy5+Jmg+XReUJqt56Z-zABK+UEswHW1dXhH-9cNw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, 2016-09-14 at 12:36 +0530, Parav Pandit wrote:
> Hi Dennis,
>
> Do you know how would HFI1 driver would work along with rdma cgroup?
Keep in mind HFI1 driver has two "modes" of operation. We support
verbs, and would surely fall in line with whatever cgroups do for IB
core. For our psm interface, not sure how cgroups would come into play.
Psm is designed to expose the hw to user and avoid the kernel when
possible adding more kernel control is sort of contrary to that.
Now that being said, Christoph recently made mention of maybe having a
drivers/psm [1]. I really haven't had a chance to think about the
implications of that, but maybe it's worth considering, after all we
have two implementations, qib and hfi1. So anyway I'm not sure we need
to be too concerned about cgroups right now as far as psm side of
things goes.
Depending how things shake out for the uAPI rewrite, or verbs 2.0 or
whatever we are calling it today things may change.
[1] http://marc.info/?l=linux-rdma&m=147401714313831&w=2
-Denny
> Hi Matan, Leon, Jason,
> Apart from HFI1, is there any other concern?
> Or Patch is good to go?
>
> 4.8 dates are close by (2 weeks) and there are two git trees involved
> (that might cause merge error to Linus) so if there are no issues, I
> would like to make request to Doug to consider it for 4.8 early on.
>
> Parav
>
> On Mon, Sep 12, 2016 at 10:37 AM, Leon Romanovsky <leon@kernel.org>
> wrote:
> > On Sun, Sep 11, 2016 at 11:52:35AM -0600, Jason Gunthorpe wrote:
> > > On Sun, Sep 11, 2016 at 07:24:45PM +0200, Christoph Hellwig
> > > wrote:
> > > > > > > I've posted some initial work toward a) a while ago, and
> > > > > > > once we
> > > > >
> > > > > Did it get merged? Do you have a pointer?
> > > >
> > > > http://www.spinics.net/lists/linux-rdma/msg31958.html
> > >
> > > Right, I remember that. Certainly the right direction
> > >
> > > > > However, everything under verbs is not straightforward. The
> > > > > files in
> > > > > userspace are not copies...
> > > > >
> > > > > user:
> > > > >
> > > > > struct ibv_query_device {
> > > > > __u32 command;
> > > > > __u16 in_words;
> > > > > __u16 out_words;
> > > > > __u64 response;
> > > > > __u64 driver_data[0];
> > > > > };
> > > > >
> > > > > kernel:
> > > > >
> > > > > struct ib_uverbs_query_device {
> > > > > __u64 response;
> > > > > __u64 driver_data[0];
> > > > > };
> > > >
> > > > We'll obviously need different strutures for the libibvers API
> > > > and the kernel interface in this case, and we'll need to figure
> > > > out
> > > > how to properly translate them. I think a cast, plus compile
> > > > time
> > > > type checking ala BUILD_BUG_ON is the way to go.
> > >
> > > I'm not sure I follow, which would I cast?
> > >
> > > BUILD_BUG_ON(sizeof(ibv_query_device) ==
> > > sizeof(ib_uverbs_cmd_hdr) +
> > > sizeof(ib_uverbs_query_device))
> > >
> > > ?
> > >
> > > > > I'm thinking the best way forward might be to use a script
> > > > > and
> > > > > transform userspace into:
> > > > >
> > > > > struct ibv_query_device {
> > > > > struct ib_uverbs_cmd_hdr hdr;
> > > > > struct ib_uverbs_query_device cmd;
> > > > > };
> > > >
> > > > That would break the users of the interface.
> > >
> > > Sorry, I mean doing this inside rdma-plumbing. Since the change
> > > is ABI
> > > identical the modified libibverbs would still be binary
> > > compatible
> > > with all providers but not source compatible. Since all kernel
> > > supported providers are in rdma-plumbing we can add the '.cmd.'
> > > at the
> > > same time.
> > >
> > > The kernel uapi header would stay the same.
> > >
> > > > However automatically generating the user ABI from the kernel
> > > > one
> > > > might still be a good idea in the long run.
> > >
> > > My preference would be to try and use the kernel headers
> > > directly.
> >
> > I thought the same, especially after realizing that they are almost
> > copy/paste from the vendor *-abi.h files.
> >
> > >
> > > Jason
> --
> 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
* Re: VERBOSE flags
From: Dalessandro, Dennis @ 2016-09-19 12:29 UTC (permalink / raw)
To: leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160919061558.GG3273-2ukJVAZIZ/Y@public.gmane.org>
On Mon, 2016-09-19 at 09:15 +0300, Leon Romanovsky wrote:
> The function pointers will cause us to create two similar functions
> (debug/regular) for every important function. It will leave to code
> duplication and/or introducing of new code to fill additional fields
> in returns to debug functions, so they will be able to print it.
I do sort of like Doug's idea. It means you can easily instrument the
heck out of a routine and have it always available. It provides more
flexibility in what you can do in a debug version.
You do make a valid point though about duplicating code. Perhaps that
can be alleviated to some extent just by refactoring and adding
inlines, etc.
> I wanted to propose to use tracepoints to provide debug information
> in
> data paths. The tracepoints infrastructure uses hooks in the code
> with
> minimal impact on performance.
This is so far the path we have chosen for hfi1. We have a number of
tracepoints in various parts of the code. Again it's available on
demand without special kernels or boot/driver options.
> > Then we can drop the CONFIG_*_DEBUG options out of all of the
> > drivers. That would be my preference. I don't want any compile
> > time
> > options because you never will be running a debug kernel when you
> > need
> > it. Per device, run time selectable so that you get 0 overhead in
> > the
> > normal case is what I would like to see.
>
> Great,
> We are aligned on this point, so do I. I see no reason in special
> compiled
> kernel or global module parameter to print debug prints for whole
> driver.
Same here.
-Denny
^ permalink raw reply
* [PATCH] IB/rxe: avoid putting a large struct rxe_qp on stack
From: Arnd Bergmann @ 2016-09-19 11:57 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, Moni Shoua, Yonatan Cohen, Leon Romanovsky,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
A race condition fix added an rxe_qp structure to the stack in order
to be able to perform rollback in rxe_requester(), but the structure
is large enough to trigger the warning for possible stack overflow:
drivers/infiniband/sw/rxe/rxe_req.c: In function 'rxe_requester':
drivers/infiniband/sw/rxe/rxe_req.c:757:1: error: the frame size of 2064 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
This changes the rollback function to only save the psn inside
the qp, which is the only field we access in the rollback_qp
anyway.
Fixes: 3050b9985024 ("IB/rxe: Fix race condition between requester and completer")
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/infiniband/sw/rxe/rxe_req.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index 13a848a518e8..cf1ffac25585 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -548,23 +548,23 @@ static void update_wqe_psn(struct rxe_qp *qp,
static void save_state(struct rxe_send_wqe *wqe,
struct rxe_qp *qp,
struct rxe_send_wqe *rollback_wqe,
- struct rxe_qp *rollback_qp)
+ u32 *rollback_psn)
{
rollback_wqe->state = wqe->state;
rollback_wqe->first_psn = wqe->first_psn;
rollback_wqe->last_psn = wqe->last_psn;
- rollback_qp->req.psn = qp->req.psn;
+ *rollback_psn = qp->req.psn;
}
static void rollback_state(struct rxe_send_wqe *wqe,
struct rxe_qp *qp,
struct rxe_send_wqe *rollback_wqe,
- struct rxe_qp *rollback_qp)
+ u32 rollback_psn)
{
wqe->state = rollback_wqe->state;
wqe->first_psn = rollback_wqe->first_psn;
wqe->last_psn = rollback_wqe->last_psn;
- qp->req.psn = rollback_qp->req.psn;
+ qp->req.psn = rollback_psn;
}
static void update_state(struct rxe_qp *qp, struct rxe_send_wqe *wqe,
@@ -593,8 +593,8 @@ int rxe_requester(void *arg)
int mtu;
int opcode;
int ret;
- struct rxe_qp rollback_qp;
struct rxe_send_wqe rollback_wqe;
+ u32 rollback_psn;
next_wqe:
if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
@@ -717,7 +717,7 @@ int rxe_requester(void *arg)
* rxe_xmit_packet().
* Otherwise, completer might initiate an unjustified retry flow.
*/
- save_state(wqe, qp, &rollback_wqe, &rollback_qp);
+ save_state(wqe, qp, &rollback_wqe, &rollback_psn);
update_wqe_state(qp, wqe, &pkt);
update_wqe_psn(qp, wqe, &pkt, payload);
ret = rxe_xmit_packet(to_rdev(qp->ibqp.device), qp, &pkt, skb);
@@ -725,7 +725,7 @@ int rxe_requester(void *arg)
qp->need_req_skb = 1;
kfree_skb(skb);
- rollback_state(wqe, qp, &rollback_wqe, &rollback_qp);
+ rollback_state(wqe, qp, &rollback_wqe, rollback_psn);
if (ret == -EAGAIN) {
rxe_run_task(&qp->req.task, 1);
--
2.9.0
--
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
* Re: [PATCH v1] ibacm: Handle EP expiration time
From: Hal Rosenstock @ 2016-09-19 11:22 UTC (permalink / raw)
To: Yuval Shaia, sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474271168-22662-1-git-send-email-yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
On 9/19/2016 3:46 AM, Yuval Shaia wrote:
> Old destination records are removed from EP's dest_map after timeout is
> expired in order to maintain a correct cache.
>
> Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> v0 -> v1:
> * Move the status and timeout check to acmp_acquire_dest (i.e.
> deleteis_dest_ready).
> * Fix typo (Record expiered).
> * Use NULL instead of 0
> ---
> prov/acmp/src/acmp.c | 29 ++++++++++++++++++++---------
> 1 files changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/prov/acmp/src/acmp.c b/prov/acmp/src/acmp.c
> index ec64631..1778ca3 100644
> --- a/prov/acmp/src/acmp.c
> +++ b/prov/acmp/src/acmp.c
> @@ -356,6 +356,16 @@ acmp_put_dest(struct acmp_dest *dest)
> }
> }
>
> +/* Caller must hold ep lock. */
> +static void
> +acmp_remove_dest(struct acmp_ep *ep, struct acmp_dest *dest)
> +{
> + acm_log(2, "%s\n", dest->name);
> + tdelete(dest->address, &ep->dest_map[dest->addr_type - 1],
> + acmp_compare_dest);
> + acmp_put_dest(dest);
> +}
> +
> static struct acmp_dest *
> acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
> {
> @@ -366,6 +376,16 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
> acm_log(2, "%s\n", log_data);
> lock_acquire(&ep->lock);
> dest = acmp_get_dest(ep, addr_type, addr);
> + if (dest && dest->state == ACMP_READY &&
> + dest->addr_timeout != 0xFFFFFFFFFFFFFFFF) {
Nit related to the timeout check:
In acmp.c, (uint64_t)~0ULL is used in other places for this. Should this
addr_timeout check be changed to some infinite or no timeout define
along with the other occurrences for consistency and better code
readability ?
-- Hal
> + acm_log(2, "Record valid for the next %ld minute(s)\n",
> + dest->addr_timeout - time_stamp_min());
> + if (time_stamp_min() >= dest->addr_timeout) {
> + acm_log(2, "Record expired\n");
> + acmp_remove_dest(ep, dest);
> + dest = NULL;
> + }
> + }
> if (!dest) {
> dest = acmp_alloc_dest(addr_type, addr);
> if (dest) {
> @@ -378,15 +398,6 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
> return dest;
> }
>
> -/* Caller must hold ep lock. */
> -//static void
> -//acmp_remove_dest(struct acmp_ep *ep, struct acmp_dest *dest)
> -//{
> -// acm_log(2, "%s\n", dest->name);
> -// tdelete(dest->address, &ep->dest_map[dest->addr_type - 1], acmp_compare_dest);
> -// acmp_put_dest(dest);
> -//}
> -
> static struct acmp_request *acmp_alloc_req(uint64_t id, struct acm_msg *msg)
> {
> struct acmp_request *req;
>
--
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 libibverbs v2 3/3] Provide remote XRC SRQ number in kernel post_send.
From: Knut Omang @ 2016-09-19 9:12 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker
In-Reply-To: <20160919052911.GF3273-2ukJVAZIZ/Y@public.gmane.org>
On Mon, 2016-09-19 at 08:29 +0300, Leon Romanovsky wrote:
> On Sat, Sep 17, 2016 at 05:59:13AM +0200, Knut Omang wrote:
> >
> > Also proper end align the ibv_kern_send_wr struct.
> >
> > Requires a corresponding kernel change.
> >
> > Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> > include/infiniband/kern-abi.h | 1 +
> > src/cmd.c | 2 ++
> > 2 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/infiniband/kern-abi.h b/include/infiniband/kern-abi.h
> > index 8bdeef5..7b1d310 100644
> > --- a/include/infiniband/kern-abi.h
> > +++ b/include/infiniband/kern-abi.h
> > @@ -800,6 +800,7 @@ struct ibv_kern_send_wr {
> > union {
> > struct {
> > __u32 remote_srqn;
> > + __u32 reserved;
> > } xrc;
> > } qp_type;
> > };
> > diff --git a/src/cmd.c b/src/cmd.c
> > index a418ee1..a4e2f75 100644
> > --- a/src/cmd.c
> > +++ b/src/cmd.c
> > @@ -1293,6 +1293,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
> > tmp->wr.ud.remote_qpn = i->wr.ud.remote_qpn;
> > tmp->wr.ud.remote_qkey = i->wr.ud.remote_qkey;
> > } else {
> > + if (ibqp->qp_type == IBV_QPT_XRC_SEND)
> > + tmp->qp_type.xrc.remote_srqn = i->qp_type.xrc.remote_srqn;
> It will be checked for all QPTs and for all consumers. Any chances to
> optimize it?
All QPTs except UD, yes.
The easiest is perhaps just to remove the test (and set the attribute in all cases), which will work fine
as long as the qp_type union is not used for anything else, but is it worth
the loss of intuitiveness/future potential correctness of the code?
Note that this is for user mode post_send implemented by the kernel, not the "fast path"
pure user mode.
Knut
> >
> > switch (i->opcode) {
> > case IBV_WR_RDMA_WRITE:
> > case IBV_WR_RDMA_WRITE_WITH_IMM:
> > --
> > git-series 0.8.10
> > --
> > 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: [RFC 06/11] Add support for QP verbs
From: Amrani, Ram @ 2016-09-19 9:00 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
Yuval.Mintz-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
Ariel.Elior-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
Michal.Kalderon-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
rajesh.borundia-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20160915130556.GA26069-2ukJVAZIZ/Y@public.gmane.org>
> > +
> > + rc = ib_get_cached_gid(ibqp->device, attr->ah_attr.port_num,
> > + attr->ah_attr.grh.sgid_index, &gid, &gid_attr);
> > + if (!rc && !memcmp(&gid, &zgid, sizeof(gid)))
> > + rc = -ENOENT;
> > +
> > + if (!rc && gid_attr.ndev) {
<snip>
> > + }
> > + }
> > + if (rc)
> > + return -EINVAL;
>
> I think it is better to check "rc" right after call to ib_get_cached_gid().
Somehow this e-mail slipped.
Thanks, will update.
--
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: [RFC 03/11] Add support for RoCE HW init
From: Amrani, Ram @ 2016-09-19 8:45 UTC (permalink / raw)
To: Sagi Grimberg
Cc: Yuval.Mintz-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
Ariel.Elior-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
Michal.Kalderon-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
rajesh.borundia-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
In-Reply-To: <29e0d88d-63f2-2b30-2856-a0e94cf31f8e@grimberg.me>
> > > + dev->max_sge = min_t(u32, RDMA_MAX_SGE_PER_SQ_WQE,
> > > + RDMA_MAX_SGE_PER_RQ_WQE);
> >
> > Our kernel target mode consumers sort of rely on max_sge_rd, you need
> > to make sure to set it too.
> Good catch. Thanks!
Actually, as I come to code this, I just realized that max_sge_rd is already set in the query verb itself:
+int qedr_query_device(struct ib_device *ibdev,
+ struct ib_device_attr *attr, struct ib_udata *udata)
+{
<snip>
+ attr->max_sge = qattr->max_sge;
+ attr->max_sge_rd = qattr->max_sge;
--
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
* [PATCH v1] ibacm: Handle EP expiration time
From: Yuval Shaia @ 2016-09-19 7:46 UTC (permalink / raw)
To: sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
Old destination records are removed from EP's dest_map after timeout is
expired in order to maintain a correct cache.
Signed-off-by: Yuval Shaia <yuval.shaia-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
v0 -> v1:
* Move the status and timeout check to acmp_acquire_dest (i.e.
deleteis_dest_ready).
* Fix typo (Record expiered).
* Use NULL instead of 0
---
prov/acmp/src/acmp.c | 29 ++++++++++++++++++++---------
1 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/prov/acmp/src/acmp.c b/prov/acmp/src/acmp.c
index ec64631..1778ca3 100644
--- a/prov/acmp/src/acmp.c
+++ b/prov/acmp/src/acmp.c
@@ -356,6 +356,16 @@ acmp_put_dest(struct acmp_dest *dest)
}
}
+/* Caller must hold ep lock. */
+static void
+acmp_remove_dest(struct acmp_ep *ep, struct acmp_dest *dest)
+{
+ acm_log(2, "%s\n", dest->name);
+ tdelete(dest->address, &ep->dest_map[dest->addr_type - 1],
+ acmp_compare_dest);
+ acmp_put_dest(dest);
+}
+
static struct acmp_dest *
acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
{
@@ -366,6 +376,16 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
acm_log(2, "%s\n", log_data);
lock_acquire(&ep->lock);
dest = acmp_get_dest(ep, addr_type, addr);
+ if (dest && dest->state == ACMP_READY &&
+ dest->addr_timeout != 0xFFFFFFFFFFFFFFFF) {
+ acm_log(2, "Record valid for the next %ld minute(s)\n",
+ dest->addr_timeout - time_stamp_min());
+ if (time_stamp_min() >= dest->addr_timeout) {
+ acm_log(2, "Record expired\n");
+ acmp_remove_dest(ep, dest);
+ dest = NULL;
+ }
+ }
if (!dest) {
dest = acmp_alloc_dest(addr_type, addr);
if (dest) {
@@ -378,15 +398,6 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t *addr)
return dest;
}
-/* Caller must hold ep lock. */
-//static void
-//acmp_remove_dest(struct acmp_ep *ep, struct acmp_dest *dest)
-//{
-// acm_log(2, "%s\n", dest->name);
-// tdelete(dest->address, &ep->dest_map[dest->addr_type - 1], acmp_compare_dest);
-// acmp_put_dest(dest);
-//}
-
static struct acmp_request *acmp_alloc_req(uint64_t id, struct acm_msg *msg)
{
struct acmp_request *req;
--
1.7.1
--
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
* Re: [PATCH] ibacm: Handle EP expiration time
From: Yuval Shaia @ 2016-09-19 7:25 UTC (permalink / raw)
To: Hefty, Sean; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB08CA2E-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
On Fri, Sep 16, 2016 at 07:38:29PM +0000, Hefty, Sean wrote:
> > +static inline is_dest_ready(struct acmp_dest *dest)
> > +{
> > + return dest->state == ACMP_READY &&
> > + dest->addr_timeout != 0xFFFFFFFFFFFFFFFF;
> > +}
>
> I found including the timeout check here to be confusing.
Checking state is not enough as there are places where add_timeout is not
set while state still is.
>
> > +
> > static struct acmp_dest *
> > acmp_acquire_dest(struct acmp_ep *ep, uint8_t addr_type, const uint8_t
> > *addr)
> > {
> > @@ -366,6 +382,15 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t
> > addr_type, const uint8_t *addr)
> > acm_log(2, "%s\n", log_data);
> > lock_acquire(&ep->lock);
> > dest = acmp_get_dest(ep, addr_type, addr);
> > + if (dest && is_dest_ready(dest)) {
>
> I think it would be clearer to just perform the state check here and either avoid the timeout check or merge it with the if statement below.
As stated above, i cannot ignore the timeout so will merge it here.
>
> > + acm_log(2, "Record valid for the next %ld minute(s)\n",
> > + dest->addr_timeout - time_stamp_min());
> > + if (time_stamp_min() >= dest->addr_timeout) {
> > + acm_log(2, "Record expiered\n");
>
> Nit: misspelled expired.
Oops, thanks!
>
> > + acmp_remove_dest(ep, dest);
> > + dest = 0;
>
> Please use NULL in place of 0.
Will do.
>
> > + }
> > + }
> > if (!dest) {
> > dest = acmp_alloc_dest(addr_type, addr);
> > if (dest) {
> > @@ -378,15 +403,6 @@ acmp_acquire_dest(struct acmp_ep *ep, uint8_t
> > addr_type, const uint8_t *addr)
> > return dest;
> > }
> >
> > -/* Caller must hold ep lock. */
> > -//static void
> > -//acmp_remove_dest(struct acmp_ep *ep, struct acmp_dest *dest)
> > -//{
> > -// acm_log(2, "%s\n", dest->name);
> > -// tdelete(dest->address, &ep->dest_map[dest->addr_type - 1],
> > acmp_compare_dest);
> > -// acmp_put_dest(dest);
> > -//}
> > -
> > static struct acmp_request *acmp_alloc_req(uint64_t id, struct acm_msg
> > *msg)
> > {
> > struct acmp_request *req;
>
> I'm fine with the email posting of the patch, but it's easier for me to deal with a github pull request. If you could add a PR as well, I'd appreciate it.
Sure, will post v1 also as PR.
>
> - Sean
> --
> 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: VERBOSE flags
From: Leon Romanovsky @ 2016-09-19 6:15 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-rdma
In-Reply-To: <0a26585a-2e54-1c24-d212-0e2469afb8d8-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]
On Sun, Sep 18, 2016 at 04:35:37PM -0400, Doug Ledford wrote:
> On 9/14/2016 2:37 AM, Leon Romanovsky wrote:
> > On Wed, Sep 14, 2016 at 9:17 AM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >> On 9/14/2016 1:27 AM, Leon Romanovsky wrote:
> >>> Hi Doug,
> >>>
> >>> Can you please respond to my questions to this patch [1]?
> >>>
> >>> Thanks
> >>>
> >>> [1] https://patchwork.kernel.org/patch/9302591/
> >>>
> >>
> >> I don't appear to still have that email. But I know what I came up with
> >> at the time.
> >>
> >> The debug information was in the mlx5 entry point for post_send. That
> >> means you can create a sysfs entry that allows you to enable or disable
> >> the debugging, and then make two different mlx5 post_send entry points.
> >> When debugging is enabled, you simply change the entry point for
> >> mlx5_post_send to mlx5_post_send_debug, where mlx5_post_send_debug is
> >> essentially nothing but mlx5_post_send; dump_wqe. The unconditional
> >> branch will be highly optimized by the CPU, so no real loss on it. And
> >> when not in debug mode, there is zero penalty. In short, make use of
> >> the entry pointers to make this configurable but also zero penalty on
> >> the hot path in the non-debug case.
> >
> > I did it in similar way to Intel's HFI driver and had in mind future
> > consolidation of all
> > these verbose flags for whole subsystem.
> >
> > I proposed topic (debuggability) in mini-summit to talk about it.
>
> OK, but if the hfi1 driver does this similar to what you were doing,
> then I think we should change it too.
>
> Essentially, given that all IB devices have a set of function pointers
> that are registered and stored on a per device basis, there is no reason
> that the core code can't be modified to present
> /sys/class/infiniband/debug/<list of specific entry points we wish to
> have possible debug functions for> and if the user wants to debug a
> specific entry point, they set it to 1 when it's normally 0. Then
> drivers can optionally register debug versions of their entry points,
> and in the core if the user tries to enable debug on an entry point, and
> the driver registered a debug variant for that entry point, we start
> using it.
As a short writeup to anyone who lacks context of this thread, we are talking
about debug information in DATA paths flows. The control paths don't need
anything special and standard dynamic_debug and/or tracing infrastructure
will be enough for them.
Doug,
The function pointers will cause us to create two similar functions
(debug/regular) for every important function. It will leave to code
duplication and/or introducing of new code to fill additional fields
in returns to debug functions, so they will be able to print it.
I wanted to propose to use tracepoints to provide debug information in
data paths. The tracepoints infrastructure uses hooks in the code with
minimal impact on performance.
> Then we can drop the CONFIG_*_DEBUG options out of all of the
> drivers. That would be my preference. I don't want any compile time
> options because you never will be running a debug kernel when you need
> it. Per device, run time selectable so that you get 0 overhead in the
> normal case is what I would like to see.
Great,
We are aligned on this point, so do I. I see no reason in special compiled
kernel or global module parameter to print debug prints for whole driver.
>
> --
> 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 net-next] chcr/cxgb4i/cxgbit/RDMA/cxgb4: Allocate resources dynamically for all cxgb4 ULD's
From: David Miller @ 2016-09-19 5:38 UTC (permalink / raw)
To: hariprasad
Cc: netdev, linux-scsi, target-devel, linux-rdma, linux-crypto, nab,
jejb, martin.petersen, dledford, herbert, varun, nirranjan, swise,
atul.gupta, rahul.lakkireddy
In-Reply-To: <1474080159-14980-1-git-send-email-hariprasad@chelsio.com>
From: Hariprasad Shenai <hariprasad@chelsio.com>
Date: Sat, 17 Sep 2016 08:12:39 +0530
> Allocate resources dynamically to cxgb4's Upper layer driver's(ULD) like
> cxgbit, iw_cxgb4 and cxgb4i. Allocate resources when they register with
> cxgb4 driver and free them while unregistering. All the queues and the
> interrupts for them will be allocated during ULD probe only and freed
> during remove.
>
> Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH libibverbs v2 3/3] Provide remote XRC SRQ number in kernel post_send.
From: Leon Romanovsky @ 2016-09-19 5:29 UTC (permalink / raw)
To: Knut Omang; +Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA, Mukesh Kacker
In-Reply-To: <d57e9dc45f6f4aed9dfb86a498226dab99e530a4.1474063039.git-series.knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]
On Sat, Sep 17, 2016 at 05:59:13AM +0200, Knut Omang wrote:
> Also proper end align the ibv_kern_send_wr struct.
>
> Requires a corresponding kernel change.
>
> Signed-off-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Mukesh Kacker <mukesh.kacker-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> include/infiniband/kern-abi.h | 1 +
> src/cmd.c | 2 ++
> 2 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/include/infiniband/kern-abi.h b/include/infiniband/kern-abi.h
> index 8bdeef5..7b1d310 100644
> --- a/include/infiniband/kern-abi.h
> +++ b/include/infiniband/kern-abi.h
> @@ -800,6 +800,7 @@ struct ibv_kern_send_wr {
> union {
> struct {
> __u32 remote_srqn;
> + __u32 reserved;
> } xrc;
> } qp_type;
> };
> diff --git a/src/cmd.c b/src/cmd.c
> index a418ee1..a4e2f75 100644
> --- a/src/cmd.c
> +++ b/src/cmd.c
> @@ -1293,6 +1293,8 @@ int ibv_cmd_post_send(struct ibv_qp *ibqp, struct ibv_send_wr *wr,
> tmp->wr.ud.remote_qpn = i->wr.ud.remote_qpn;
> tmp->wr.ud.remote_qkey = i->wr.ud.remote_qkey;
> } else {
> + if (ibqp->qp_type == IBV_QPT_XRC_SEND)
> + tmp->qp_type.xrc.remote_srqn = i->qp_type.xrc.remote_srqn;
It will be checked for all QPTs and for all consumers. Any chances to
optimize it?
> switch (i->opcode) {
> case IBV_WR_RDMA_WRITE:
> case IBV_WR_RDMA_WRITE_WITH_IMM:
> --
> git-series 0.8.10
> --
> 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 0/2] cxgb4 FR_NSMR_TPTE_WR support
From: David Miller @ 2016-09-19 5:16 UTC (permalink / raw)
To: swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1474037695.git.swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
From: Steve Wise <swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW@public.gmane.org>
Date: Fri, 16 Sep 2016 07:54:55 -0700
> This series enables a new work request to optimize small REG_MR
> operations. This is intended for 4.9. If everyone agrees, I suggest
> Doug take both the cxgb4 and iw_cxgb4 patches through his tree.
I'm assuming this mean that I do _not_ apply these to my tree.
--
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