Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* Re: failed armel build of rdma-core 24.0-1
From: Gal Pressman @ 2019-07-11  8:00 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Jason Gunthorpe, Benjamin Drung, linux-rdma
In-Reply-To: <20190711070744.GD23598@mtr-leonro.mtl.com>

On 11/07/2019 10:07, Leon Romanovsky wrote:
> On Thu, Jul 11, 2019 at 09:35:59AM +0300, Gal Pressman wrote:
>> On 10/07/2019 22:23, Jason Gunthorpe wrote:
>>> Benjamin,
>>>
>>> Can you confirm that something like this fixes these build problems?
>>>
>>> diff --git a/debian/rules b/debian/rules
>>> index 07c9c145ff090c..facb45170eacfc 100755
>>> --- a/debian/rules
>>> +++ b/debian/rules
>>> @@ -63,6 +63,7 @@ ifneq (,$(filter-out $(COHERENT_DMA_ARCHS),$(DEB_HOST_ARCH)))
>>>  		test -e debian/$$package.install.backup || cp debian/$$package.install debian/$$package.install.backup; \
>>>  	done
>>>  	sed -i '/mlx[45]/d' debian/ibverbs-providers.install debian/libibverbs-dev.install debian/rdma-core.install
>>> +	sed -i '/efa/d' debian/ibverbs-providers.install debian/libibverbs-dev.install debian/rdma-core.install
>>>  endif
>>>  	DESTDIR=$(CURDIR)/debian/tmp ninja -C build-deb install
>>>
>>>
>>>
>>> On Wed, Jul 10, 2019 at 11:37:01AM -0000, Debian buildds wrote:
>>>>  * Source package: rdma-core
>>>>  * Version: 24.0-1
>>>>  * Architecture: armel
>>>>  * State: failed
>>>>  * Suite: sid
>>>>  * Builder: antheil.debian.org
>>>>  * Build log: https://buildd.debian.org/status/fetch.php?pkg=rdma-core&arch=armel&ver=24.0-1&stamp=1562758620&file=log
>>>>
>>>> Please note that these notifications do not necessarily mean bug reports
>>>> in your package but could also be caused by other packages, temporary
>>>> uninstallabilities and arch-specific breakages.  A look at the build log
>>>> despite this disclaimer would be appreciated however.
>>
>> Was this error supposed to be reported by travis as well? I'm pretty sure all
>> tests passed.
> 
> Did you get any build failure email from debian? I didn't.

I didn't, who is supposed to get these?

^ permalink raw reply

* Re: [PATCH] rdma/siw: Use proper enumerated type in map_cqe_status
From: Nathan Chancellor @ 2019-07-11  8:14 UTC (permalink / raw)
  To: Bernard Metzler
  Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel,
	clang-built-linux
In-Reply-To: <OFE93E0F86.E35CE856-ON00258434.002A83CE-00258434.002A83DF@notes.na.collabserv.com>

Hi Bernard,

On Thu, Jul 11, 2019 at 07:44:22AM +0000, Bernard Metzler wrote:
> Nathan, thanks very much. That's correct.

Thanks for the confirmation that the fix was correct.

> I don't know how this could pass w/o warning.

Unfortunately, it appears that GCC only warns when two different
enumerated types are directly compared, not when they are implicitly
converted between.

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78736

If it did, I wouldn't have fixed as many warnings as I have.

https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+is%3Aclosed+label%3A-Wenum-conversion

Maybe time to start plumbing Clang into your test flow until it can get
intergrated with more CI setups? :) It can catch some pretty dodgy
behavior that GCC doesn't:

https://github.com/ClangBuiltLinux/linux/issues/390

https://github.com/ClangBuiltLinux/linux/issues/544

Kernel CI has added support for it (although they don't email the
authors of patches individually) and 0day is currently working on it.
Feel free to reach out if you decide to explore it, I'm always happy
to help.

Cheers,
Nathan

^ permalink raw reply

* Re:  [PATCH -next] rdma/siw: remove set but not used variable 's'
From: Bernard Metzler @ 2019-07-11  8:22 UTC (permalink / raw)
  To: YueHaibing; +Cc: dledford, jgg, linux-kernel, linux-rdma
In-Reply-To: <20190711071213.57880-1-yuehaibing@huawei.com>

-----"YueHaibing" <yuehaibing@huawei.com> wrote: -----

>To: <bmt@zurich.ibm.com>, <dledford@redhat.com>, <jgg@ziepe.ca>
>From: "YueHaibing" <yuehaibing@huawei.com>
>Date: 07/11/2019 09:13AM
>Cc: <linux-kernel@vger.kernel.org>, <linux-rdma@vger.kernel.org>,
>"YueHaibing" <yuehaibing@huawei.com>
>Subject: [EXTERNAL] [PATCH -next] rdma/siw: remove set but not used
>variable 's'
>
>Fixes gcc '-Wunused-but-set-variable' warning:
>
>drivers/infiniband/sw/siw/siw_cm.c: In function
>siw_cm_llp_state_change:
>drivers/infiniband/sw/siw/siw_cm.c:1278:17: warning: variable s set
>but not used [-Wunused-but-set-variable]
>
>Reported-by: Hulk Robot <hulkci@huawei.com>
>Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>---
> drivers/infiniband/sw/siw/siw_cm.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/infiniband/sw/siw/siw_cm.c
>b/drivers/infiniband/sw/siw/siw_cm.c
>index c883bf5..7d87a78 100644
>--- a/drivers/infiniband/sw/siw/siw_cm.c
>+++ b/drivers/infiniband/sw/siw/siw_cm.c
>@@ -1275,7 +1275,6 @@ static void siw_cm_llp_error_report(struct sock
>*sk)
> static void siw_cm_llp_state_change(struct sock *sk)
> {
> 	struct siw_cep *cep;
>-	struct socket *s;
> 	void (*orig_state_change)(struct sock *s);
> 
> 	read_lock(&sk->sk_callback_lock);
>@@ -1288,8 +1287,6 @@ static void siw_cm_llp_state_change(struct sock
>*sk)
> 	}
> 	orig_state_change = cep->sk_state_change;
> 
>-	s = sk->sk_socket;
>-
> 	siw_dbg_cep(cep, "state: %d\n", cep->state);
> 
> 	switch (sk->sk_state) {
>-- 
>2.7.4
>
>
>

Another bad leftover from excessive debugging times...

Thanks alot Yue!
Bernard.


^ permalink raw reply

* Re: [PATCH v4 00/25] InfiniBand Transport (IBTRS) and Network Block Device (IBNBD)
From: Danil Kipnis @ 2019-07-11  8:54 UTC (permalink / raw)
  To: Sagi Grimberg
  Cc: Jack Wang, linux-block, linux-rdma, axboe, Christoph Hellwig,
	bvanassche, jgg, dledford, Roman Pen, gregkh
In-Reply-To: <a8f2f1d2-b5d9-92fc-40c8-090af0487723@grimberg.me>

Hi Sagi,

thanks a lot for the detailed reply. Answers inline below:

On Tue, Jul 9, 2019 at 9:46 PM Sagi Grimberg <sagi@grimberg.me> wrote:
>
> Hi Danil and Jack,
>
> > Hallo Doug, Hallo Jason, Hallo Jens, Hallo Greg,
> >
> > Could you please provide some feedback to the IBNBD driver and the
> > IBTRS library?
> > So far we addressed all the requests provided by the community
>
> That is not exactly correct AFAIR,
>
> My main issues which were raised before are:
> - IMO there isn't any justification to this ibtrs layering separation
>    given that the only user of this is your ibnbd. Unless you are
>    trying to submit another consumer, you should avoid adding another
>    subsystem that is not really general purpose.
We designed ibtrs not only with the IBNBD in mind but also as the
transport layer for a distributed SDS. We'd like to be able to do what
ceph is capable of (automatic up/down scaling of the storage cluster,
automatic recovery) but using in-kernel rdma-based IO transport
drivers, thin-provisioned volume managers, etc. to keep the highest
possible performance. That modest plan of ours should among others
cover for the following:
When using IBNBD/SRP/NVMEoF to export devices (say, thin-provisioned
volumes) from server to client and building an (md-)raid on top of the
imported devices on client side in order to provide for redundancy
across different machines, one gets very decent throughput and low
latency, since the IOs are sent in parallel to the storage machines.
One downside of this setup, is that the resync traffic has to flow
over the client, where the md-raid is sitting. Ideally the resync
traffic should flow directly between the two "legs" (storage machines)
of the raid. The server side of such a "distributed raid" capable of
this direct syncing between the array members would necessarily
require to have some logic on server side and hence could also sit on
top of ibtrs. (To continue the analogy, the "server" side of an
md-raid build on top of say two NVMEoF devices are just two block
devices, which couldn't communicate with each other)
All in all itbrs is a library to establish a "fat", multipath,
autoreconnectable connection between two hosts on top of rdma,
optimized for transport of IO traffic.

> - ibtrs in general is using almost no infrastructure from the existing
>    kernel subsystems. Examples are:
>    - tag allocation mechanism (which I'm not clear why its needed)
As you correctly noticed our client manages the buffers allocated and
registered by the server on the connection establishment. Our tags are
just a mechanism to take and release those buffers for incoming
requests on client side. Since the buffers allocated by the server are
to be shared between all the devices mapped from that server and all
their HW queues (each having num_cpus of them) the mechanism behind
get_tag/put_tag also takes care of the fairness.

>    - rdma rw abstraction similar to what we have in the core
On the one hand we have only single IO related function:
ibtrs_clt_request(READ/WRITE, session,...), which executes rdma write
with imm, or requests an rdma write with imm to be executed by the
server. On the other hand we provide an abstraction to establish and
manage what we call "session", which consist of multiple paths (to do
failover and multipath with different policies), where each path
consists of num_cpu rdma connections. Once you established a session
you can add or remove paths from it on the fly. In case the connection
to server is lost, the client does periodic attempts to reconnect
automatically. On the server side you get just sg-lists with a
direction READ or WRITE as requested by the client. We designed this
interface not only as the minimum required to build a block device on
top of rdma but also with a distributed raid in mind.

>    - list_next_or_null_rr_rcu ??
We use that for multipath. The macro (and more importantly the way we
use it) has been reviewed by Linus and quit closely by Paul E.
McKenney. AFAIR the conclusion was that Romans implementation is
correct, but too tricky to use correctly in order to be included into
kernel as a public interface. See https://lkml.org/lkml/2018/5/18/659

>    - few other examples sprinkled around..
To my best knowledge we addressed everything we got comments on and
will definitely do so in the future.

> Another question, from what I understand from the code, the client
> always rdma_writes data on writes (with imm) from a remote pool of
> server buffers dedicated to it. Essentially all writes are immediate (no
> rdma reads ever). How is that different than using send wrs to a set of
> pre-posted recv buffers (like all others are doing)? Is it faster?
At the very beginning of the project we did some measurements and saw,
that it is faster. I'm not sure if this is still true, since the
hardware and the drivers and rdma subsystem did change in that time.
Also it seemed to make the code simpler.

> Also, given that the server pre-allocate a substantial amount of memory
> for each connection, is it documented the requirements from the server
> side? Usually kernel implementations (especially upstream ones) will
> avoid imposing such large longstanding memory requirements on the system
> by default. I don't have a firm stand on this, but wanted to highlight
> this as you are sending this for upstream inclusion.
We definitely need to stress that somewhere. Will include into readme
and add to the cover letter next time. Our memory management is indeed
basically absent in favor of performance: The server reserves
queue_depth of say 512K buffers. Each buffer is used by client for
single IO only, no matter how big the request is. So if client only
issues 4K IOs, we do waste 508*queue_depth K of memory. We were aiming
for lowest possible latency from the beginning. It is probably
possible to implement some clever allocator on the server side which
wouldn't affect the performance a lot.

>
>   and
> > continue to maintain our code up-to-date with the upstream kernel
> > while having an extra compatibility layer for older kernels in our
> > out-of-tree repository.
>
> Overall, while I absolutely support your cause to lower your maintenance
> overhead by having this sit upstream, I don't see why this can be
> helpful to anyone else in the rdma community. If instead you can
> crystallize why/how ibnbd is faster than anything else, and perhaps
> contribute a common infrastructure piece (or enhance an existing one)
> such that other existing ulps can leverage, it will be a lot more
> compelling to include it upstream.
>
> > I understand that SRP and NVMEoF which are in the kernel already do
> > provide equivalent functionality for the majority of the use cases.
> > IBNBD on the other hand is showing higher performance and more
> > importantly includes the IBTRS - a general purpose library to
> > establish connections and transport BIO-like read/write sg-lists over
> > RDMA,
>
> But who needs it? Can other ulps use it or pieces of it? I keep failing
> to understand why is this a benefit if its specific to your ibnbd?
See above and please ask if you have more questions to this.

Thank you,
Danil.

^ permalink raw reply

* Re: failed armel build of rdma-core 24.0-1
From: Benjamin Drung @ 2019-07-11 10:53 UTC (permalink / raw)
  To: Jason Gunthorpe, Gal Pressman; +Cc: linux-rdma
In-Reply-To: <20190710192316.GH4051@ziepe.ca>

Am Mittwoch, den 10.07.2019, 16:23 -0300 schrieb Jason Gunthorpe:
> Benjamin,
> 
> Can you confirm that something like this fixes these build problems?
> 
> diff --git a/debian/rules b/debian/rules
> index 07c9c145ff090c..facb45170eacfc 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -63,6 +63,7 @@ ifneq (,$(filter-out
> $(COHERENT_DMA_ARCHS),$(DEB_HOST_ARCH)))
>  		test -e debian/$$package.install.backup || cp
> debian/$$package.install debian/$$package.install.backup; \
>  	done
>  	sed -i '/mlx[45]/d' debian/ibverbs-providers.install
> debian/libibverbs-dev.install debian/rdma-core.install
> +	sed -i '/efa/d' debian/ibverbs-providers.install
> debian/libibverbs-dev.install debian/rdma-core.install
>  endif
>  	DESTDIR=$(CURDIR)/debian/tmp ninja -C build-deb install

I successfully tested a shorter version of this change on a Debian
armhf porter box and uploaded it as version 24.0-2 to Debian unstable.
Here is the pull request with all the packaging changes:
https://github.com/linux-rdma/rdma-core/pull/551

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

1&1 IONOS Cloud GmbH | Greifswalder Str. 207 | 10405 Berlin | Germany
E-mail: benjamin.drung@cloud.ionos.com | Web: www.ionos.de

Head Office: Berlin, Germany
District Court Berlin Charlottenburg, Registration number: HRB 125506 B
Executive Management: Christoph Steffens, Matthias Steinberg, Achim
Weiss

Member of United Internet


^ permalink raw reply

* Re: failed armel build of rdma-core 24.0-1
From: Leon Romanovsky @ 2019-07-11 11:11 UTC (permalink / raw)
  To: Gal Pressman; +Cc: Jason Gunthorpe, Benjamin Drung, linux-rdma
In-Reply-To: <bde8ef5c-10d4-0eae-efbf-5addff1633ff@amazon.com>

On Thu, Jul 11, 2019 at 11:00:44AM +0300, Gal Pressman wrote:
> On 11/07/2019 10:07, Leon Romanovsky wrote:
> > On Thu, Jul 11, 2019 at 09:35:59AM +0300, Gal Pressman wrote:
> >> On 10/07/2019 22:23, Jason Gunthorpe wrote:
> >>> Benjamin,
> >>>
> >>> Can you confirm that something like this fixes these build problems?
> >>>
> >>> diff --git a/debian/rules b/debian/rules
> >>> index 07c9c145ff090c..facb45170eacfc 100755
> >>> --- a/debian/rules
> >>> +++ b/debian/rules
> >>> @@ -63,6 +63,7 @@ ifneq (,$(filter-out $(COHERENT_DMA_ARCHS),$(DEB_HOST_ARCH)))
> >>>  		test -e debian/$$package.install.backup || cp debian/$$package.install debian/$$package.install.backup; \
> >>>  	done
> >>>  	sed -i '/mlx[45]/d' debian/ibverbs-providers.install debian/libibverbs-dev.install debian/rdma-core.install
> >>> +	sed -i '/efa/d' debian/ibverbs-providers.install debian/libibverbs-dev.install debian/rdma-core.install
> >>>  endif
> >>>  	DESTDIR=$(CURDIR)/debian/tmp ninja -C build-deb install
> >>>
> >>>
> >>>
> >>> On Wed, Jul 10, 2019 at 11:37:01AM -0000, Debian buildds wrote:
> >>>>  * Source package: rdma-core
> >>>>  * Version: 24.0-1
> >>>>  * Architecture: armel
> >>>>  * State: failed
> >>>>  * Suite: sid
> >>>>  * Builder: antheil.debian.org
> >>>>  * Build log: https://buildd.debian.org/status/fetch.php?pkg=rdma-core&arch=armel&ver=24.0-1&stamp=1562758620&file=log
> >>>>
> >>>> Please note that these notifications do not necessarily mean bug reports
> >>>> in your package but could also be caused by other packages, temporary
> >>>> uninstallabilities and arch-specific breakages.  A look at the build log
> >>>> despite this disclaimer would be appreciated however.
> >>
> >> Was this error supposed to be reported by travis as well? I'm pretty sure all
> >> tests passed.
> >
> > Did you get any build failure email from debian? I didn't.
>
> I didn't, who is supposed to get these?

I don't know, but expected to see build failure emails to linux-rdma@.

Thanks

^ permalink raw reply

* Re: [PATCH] rdma/siw: Use proper enumerated type in map_cqe_status
From: Jason Gunthorpe @ 2019-07-11 13:39 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Bernard Metzler, Doug Ledford, linux-rdma, linux-kernel,
	clang-built-linux
In-Reply-To: <20190711081434.GA86557@archlinux-threadripper>

On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> Hi Bernard,
> 
> On Thu, Jul 11, 2019 at 07:44:22AM +0000, Bernard Metzler wrote:
> > Nathan, thanks very much. That's correct.
> 
> Thanks for the confirmation that the fix was correct.
> 
> > I don't know how this could pass w/o warning.
> 
> Unfortunately, it appears that GCC only warns when two different
> enumerated types are directly compared, not when they are implicitly
> converted between.
> 
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wenum-compare
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78736
> 
> If it did, I wouldn't have fixed as many warnings as I have.
> 
> https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+is%3Aclosed+label%3A-Wenum-conversion
> 
> Maybe time to start plumbing Clang into your test flow until it can get
> intergrated with more CI setups? :) It can catch some pretty dodgy
> behavior that GCC doesn't:

I keep asking how to use clang to build the kernel and last I was told
it still wasn't ready..

Is it ready now? Is there some flow that will compile with clang
warning free, on any arch? (at least the portion of the kernel I check)

> Kernel CI has added support for it (although they don't email the
> authors of patches individually) 

Well.. we didn't see any emails till yours, so if others are looking
they aren't communicating?

Jason

^ permalink raw reply

* [PATCH -next] RDMA/core: fix -Wunused-const-variable warnings
From: Qian Cai @ 2019-07-11 13:55 UTC (permalink / raw)
  To: jgg; +Cc: leonro, saeedm, talgi, yaminf, linux-rdma, linux-kernel, Qian Cai

The linux-next commit "linux/dim: Implement RDMA adaptive moderation
(DIM)" [1] introduced a few compilation warnings.

In file included from ./include/rdma/ib_verbs.h:64,
                 from ./include/linux/mlx5/device.h:37,
                 from ./include/linux/mlx5/driver.h:51,
                 from drivers/net/ethernet/mellanox/mlx5/core/uar.c:36:
./include/linux/dim.h:378:1: warning: 'rdma_dim_prof' defined but not
used [-Wunused-const-variable=]
 rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
 ^~~~~~~~~~~~~
In file included from ./include/rdma/ib_verbs.h:64,
                 from ./include/linux/mlx5/device.h:37,
                 from ./include/linux/mlx5/driver.h:51,
                 from
drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:37:
./include/linux/dim.h:378:1: warning: 'rdma_dim_prof' defined but not
used [-Wunused-const-variable=]
 rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
 ^~~~~~~~~~~~~

Since only ib_cq_rdma_dim_work() in drivers/infiniband/core/cq.c uses
it, just move the definition over there.

[1] https://patchwork.kernel.org/patch/11031455/

Signed-off-by: Qian Cai <cai@lca.pw>
---
 drivers/infiniband/core/cq.c | 13 +++++++++++++
 include/linux/dim.h          | 13 -------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/core/cq.c b/drivers/infiniband/core/cq.c
index ffd6e24109d5..7c599878ccf7 100644
--- a/drivers/infiniband/core/cq.c
+++ b/drivers/infiniband/core/cq.c
@@ -18,6 +18,19 @@
 #define IB_POLL_FLAGS \
 	(IB_CQ_NEXT_COMP | IB_CQ_REPORT_MISSED_EVENTS)
 
+static const struct dim_cq_moder
+rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
+	{1,   0, 1,  0},
+	{1,   0, 4,  0},
+	{2,   0, 4,  0},
+	{2,   0, 8,  0},
+	{4,   0, 8,  0},
+	{16,  0, 8,  0},
+	{16,  0, 16, 0},
+	{32,  0, 16, 0},
+	{32,  0, 32, 0},
+};
+
 static void ib_cq_rdma_dim_work(struct work_struct *w)
 {
 	struct dim *dim = container_of(w, struct dim, work);
diff --git a/include/linux/dim.h b/include/linux/dim.h
index aa69730c3b8d..d3a0fbfff2bb 100644
--- a/include/linux/dim.h
+++ b/include/linux/dim.h
@@ -374,19 +374,6 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
 #define RDMA_DIM_PARAMS_NUM_PROFILES 9
 #define RDMA_DIM_START_PROFILE 0
 
-static const struct dim_cq_moder
-rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
-	{1,   0, 1,  0},
-	{1,   0, 4,  0},
-	{2,   0, 4,  0},
-	{2,   0, 8,  0},
-	{4,   0, 8,  0},
-	{16,  0, 8,  0},
-	{16,  0, 16, 0},
-	{32,  0, 16, 0},
-	{32,  0, 32, 0},
-};
-
 /**
  * rdma_dim - Runs the adaptive moderation.
  * @dim: The moderation struct.
-- 
1.8.3.1


^ permalink raw reply related

* Re: [PATCH -next] rdma/siw: Add missing dependencies on LIBCRC32C and DMA_VIRT_OPS
From: Jason Gunthorpe @ 2019-07-11 14:47 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Bernard Metzler, Doug Ledford, linux-rdma, linux-kernel,
	linux-next
In-Reply-To: <20190710133930.26591-1-geert@linux-m68k.org>

On Wed, Jul 10, 2019 at 03:39:30PM +0200, Geert Uytterhoeven wrote:
> If LIBCRC32C and DMA_VIRT_OPS are not enabled:
> 
>     drivers/infiniband/sw/siw/siw_main.o: In function `siw_newlink':
>     siw_main.c:(.text+0x35c): undefined reference to `dma_virt_ops'
>     drivers/infiniband/sw/siw/siw_qp_rx.o: In function `siw_csum_update':
>     siw_qp_rx.c:(.text+0x16): undefined reference to `crc32c'
> 
> Fix the first issue by adding a select of DMA_VIRT_OPS.
> Fix the second issue by replacing the unneeded dependency on
> CRYPTO_CRC32 by a dependency on LIBCRC32C.
> 
> Reported-by: noreply@ellerman.id.au (first issue)
> Fixes: c0cf5bdde46c664d ("rdma/siw: addition to kernel build environment")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
>  drivers/infiniband/sw/siw/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* Re: [PATCH -next] rdma/siw: remove set but not used variable 's'
From: Jason Gunthorpe @ 2019-07-11 14:47 UTC (permalink / raw)
  To: YueHaibing; +Cc: bmt, dledford, linux-kernel, linux-rdma
In-Reply-To: <20190711071213.57880-1-yuehaibing@huawei.com>

On Thu, Jul 11, 2019 at 03:12:13PM +0800, YueHaibing wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/infiniband/sw/siw/siw_cm.c: In function siw_cm_llp_state_change:
> drivers/infiniband/sw/siw/siw_cm.c:1278:17: warning: variable s set but not used [-Wunused-but-set-variable]
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/infiniband/sw/siw/siw_cm.c | 3 ---
>  1 file changed, 3 deletions(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* Re: [PATCH -next] RDMA/core: fix -Wunused-const-variable warnings
From: Jason Gunthorpe @ 2019-07-11 14:54 UTC (permalink / raw)
  To: Qian Cai; +Cc: leonro, saeedm, talgi, yaminf, linux-rdma, linux-kernel
In-Reply-To: <1562853356-11595-1-git-send-email-cai@lca.pw>

On Thu, Jul 11, 2019 at 09:55:56AM -0400, Qian Cai wrote:
> The linux-next commit "linux/dim: Implement RDMA adaptive moderation
> (DIM)" [1] introduced a few compilation warnings.
> 
> In file included from ./include/rdma/ib_verbs.h:64,
>                  from ./include/linux/mlx5/device.h:37,
>                  from ./include/linux/mlx5/driver.h:51,
>                  from drivers/net/ethernet/mellanox/mlx5/core/uar.c:36:
> ./include/linux/dim.h:378:1: warning: 'rdma_dim_prof' defined but not
> used [-Wunused-const-variable=]
>  rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
>  ^~~~~~~~~~~~~
> In file included from ./include/rdma/ib_verbs.h:64,
>                  from ./include/linux/mlx5/device.h:37,
>                  from ./include/linux/mlx5/driver.h:51,
>                  from
> drivers/net/ethernet/mellanox/mlx5/core/pagealloc.c:37:
> ./include/linux/dim.h:378:1: warning: 'rdma_dim_prof' defined but not
> used [-Wunused-const-variable=]
>  rdma_dim_prof[RDMA_DIM_PARAMS_NUM_PROFILES] = {
>  ^~~~~~~~~~~~~
> 
> Since only ib_cq_rdma_dim_work() in drivers/infiniband/core/cq.c uses
> it, just move the definition over there.
> 
> [1] https://patchwork.kernel.org/patch/11031455/
> 
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>  drivers/infiniband/core/cq.c | 13 +++++++++++++
>  include/linux/dim.h          | 13 -------------
>  2 files changed, 13 insertions(+), 13 deletions(-)

Applied to for-next, thanks

Jason

^ permalink raw reply

* [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Leon Romanovsky @ 2019-07-11 15:31 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Yamin Friedman

From: Leon Romanovsky <leonro@mellanox.com>

Multiply by 100 can potentially overflow cpms value and will produce
incorrect wrong ratio statistics. Update code to use built-in division
macro, so it will fix the following UBSAN warning.

 [ 1040.120129] ================================================================================
 [ 1040.127124] UBSAN: Undefined behaviour in lib/dim/dim.c:78:23
 [ 1040.130118] signed integer overflow:
 [ 1040.131643] 134718714 * 100 cannot be represented in type 'int'
 [ 1040.134374] CPU: 0 PID: 22846 Comm: iperf3 Not tainted 5.2.0-rc6-for-upstream-dbg-2019-06-29_03-18-13-29 #1
 [ 1040.139068] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
 [ 1040.144469] Call Trace:
 [ 1040.145897]  <IRQ>
 [ 1040.147366]  dump_stack+0x9a/0xeb
 [ 1040.149061]  ubsan_epilogue+0x9/0x7c
 [ 1040.150462]  handle_overflow+0x16d/0x198
 [ 1040.151911]  ? __ubsan_handle_negate_overflow+0x15c/0x15c
 [ 1040.153679]  ? sk_free+0x15/0x30
 [ 1040.155011]  ? kvm_clock_read+0x14/0x30
 [ 1040.156433]  ? kvm_sched_clock_read+0x5/0x10
 [ 1040.157952]  ? sched_clock+0x5/0x10
 [ 1040.159318]  ? sched_clock_cpu+0x18/0x260
 [ 1040.160801]  dim_calc_stats+0x4a1/0x4c0
 [ 1040.162274]  net_dim+0x147/0x920
 [ 1040.163592]  ? net_dim_stats_compare+0x330/0x330
 [ 1040.165283]  mlx5e_napi_poll+0x410/0x1030 [mlx5_core]
 [ 1040.166876]  ? lock_stats+0xd41/0x1740
 [ 1040.168266]  ? mlx5e_trigger_irq+0x550/0x550 [mlx5_core]
 [ 1040.169918]  ? __module_text_address+0x13/0x140
 [ 1040.171409]  ? lock_stats+0xd41/0x1740
 [ 1040.172757]  ? net_rx_action+0x262/0xda0
 [ 1040.174156]  net_rx_action+0x421/0xda0
 [ 1040.175519]  ? napi_complete_done+0x370/0x370
 [ 1040.176979]  ? kvm_clock_read+0x14/0x30
 [ 1040.178316]  ? kvm_sched_clock_read+0x5/0x10
 [ 1040.179690]  ? sched_clock+0x5/0x10
 [ 1040.180920]  ? sched_clock_cpu+0x18/0x260
 [ 1040.182286]  __do_softirq+0x287/0xb4e
 [ 1040.183581]  ? irqtime_account_irq+0x1d5/0x3b0
 [ 1040.184998]  irq_exit+0x17d/0x1d0
 [ 1040.186212]  do_IRQ+0x129/0x220
 [ 1040.187412]  common_interrupt+0xf/0xf
 [ 1040.188673]  </IRQ>
 [ 1040.189685] RIP: 0033:0x7f092c41a07a
 [ 1040.190884] Code: 45 31 f6 e9 8a 00 00 00 0f 1f 84 00 00 00 00 00 48
89 df ff 93 88 01 00 00 85 c0 0f 88 c7 00 00 00 48 98 48 01 85 88 02 00
00 <48> 8b 85 c8 02 00 00 48 83 85 90 02 00 00 01 48 83 78 10 00 74 0b
 [ 1040.195584] RSP: 002b:00007fffbebe7870 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffd7
 [ 1040.197933] RAX: 0000000000020000 RBX: 0000000000e239b0 RCX: 000000000006b280
 [ 1040.199740] RDX: 0000000000020000 RSI: 00007f092c805000 RDI: 0000000000000007
 [ 1040.201525] RBP: 0000000000e21260 R08: 0000000000000000 R09: 00007fffbebfb0a0
 [ 1040.203237] R10: 0000000000000380 R11: 0000000000000246 R12: 00007fffbebe7950
 [ 1040.204944] R13: 0000000000000007 R14: 0000000000000001 R15: 00007fffbebe7870
 [ 1040.206686] ================================================================================

Fixes: 398c2b05bbee ("linux/dim: Add completions count to dim_sample")
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 lib/dim/dim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/dim/dim.c b/lib/dim/dim.c
index 439d641ec796..38045d6d0538 100644
--- a/lib/dim/dim.c
+++ b/lib/dim/dim.c
@@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
 					delta_us);
 	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
 	if (curr_stats->epms != 0)
-		curr_stats->cpe_ratio =
-				(curr_stats->cpms * 100) / curr_stats->epms;
+		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
+			curr_stats->cpms * 100, curr_stats->epms);
 	else
 		curr_stats->cpe_ratio = 0;

--
2.20.1


^ permalink raw reply related

* Re: [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Jason Gunthorpe @ 2019-07-11 15:43 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yamin Friedman
In-Reply-To: <20190711153118.14635-1-leon@kernel.org>

On Thu, Jul 11, 2019 at 06:31:18PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Multiply by 100 can potentially overflow cpms value and will produce
> incorrect wrong ratio statistics. Update code to use built-in division
> macro, so it will fix the following UBSAN warning.
> 
>  [ 1040.120129] ================================================================================
>  [ 1040.127124] UBSAN: Undefined behaviour in lib/dim/dim.c:78:23
>  [ 1040.130118] signed integer overflow:
>  [ 1040.131643] 134718714 * 100 cannot be represented in type 'int'
>  [ 1040.134374] CPU: 0 PID: 22846 Comm: iperf3 Not tainted 5.2.0-rc6-for-upstream-dbg-2019-06-29_03-18-13-29 #1
>  [ 1040.139068] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
>  [ 1040.144469] Call Trace:
>  [ 1040.145897]  <IRQ>
>  [ 1040.147366]  dump_stack+0x9a/0xeb
>  [ 1040.149061]  ubsan_epilogue+0x9/0x7c
>  [ 1040.150462]  handle_overflow+0x16d/0x198
>  [ 1040.151911]  ? __ubsan_handle_negate_overflow+0x15c/0x15c
>  [ 1040.153679]  ? sk_free+0x15/0x30
>  [ 1040.155011]  ? kvm_clock_read+0x14/0x30
>  [ 1040.156433]  ? kvm_sched_clock_read+0x5/0x10
>  [ 1040.157952]  ? sched_clock+0x5/0x10
>  [ 1040.159318]  ? sched_clock_cpu+0x18/0x260
>  [ 1040.160801]  dim_calc_stats+0x4a1/0x4c0
>  [ 1040.162274]  net_dim+0x147/0x920
>  [ 1040.163592]  ? net_dim_stats_compare+0x330/0x330
>  [ 1040.165283]  mlx5e_napi_poll+0x410/0x1030 [mlx5_core]
>  [ 1040.166876]  ? lock_stats+0xd41/0x1740
>  [ 1040.168266]  ? mlx5e_trigger_irq+0x550/0x550 [mlx5_core]
>  [ 1040.169918]  ? __module_text_address+0x13/0x140
>  [ 1040.171409]  ? lock_stats+0xd41/0x1740
>  [ 1040.172757]  ? net_rx_action+0x262/0xda0
>  [ 1040.174156]  net_rx_action+0x421/0xda0
>  [ 1040.175519]  ? napi_complete_done+0x370/0x370
>  [ 1040.176979]  ? kvm_clock_read+0x14/0x30
>  [ 1040.178316]  ? kvm_sched_clock_read+0x5/0x10
>  [ 1040.179690]  ? sched_clock+0x5/0x10
>  [ 1040.180920]  ? sched_clock_cpu+0x18/0x260
>  [ 1040.182286]  __do_softirq+0x287/0xb4e
>  [ 1040.183581]  ? irqtime_account_irq+0x1d5/0x3b0
>  [ 1040.184998]  irq_exit+0x17d/0x1d0
>  [ 1040.186212]  do_IRQ+0x129/0x220
>  [ 1040.187412]  common_interrupt+0xf/0xf
>  [ 1040.188673]  </IRQ>
>  [ 1040.189685] RIP: 0033:0x7f092c41a07a
>  [ 1040.190884] Code: 45 31 f6 e9 8a 00 00 00 0f 1f 84 00 00 00 00 00 48
> 89 df ff 93 88 01 00 00 85 c0 0f 88 c7 00 00 00 48 98 48 01 85 88 02 00
> 00 <48> 8b 85 c8 02 00 00 48 83 85 90 02 00 00 01 48 83 78 10 00 74 0b
>  [ 1040.195584] RSP: 002b:00007fffbebe7870 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffd7
>  [ 1040.197933] RAX: 0000000000020000 RBX: 0000000000e239b0 RCX: 000000000006b280
>  [ 1040.199740] RDX: 0000000000020000 RSI: 00007f092c805000 RDI: 0000000000000007
>  [ 1040.201525] RBP: 0000000000e21260 R08: 0000000000000000 R09: 00007fffbebfb0a0
>  [ 1040.203237] R10: 0000000000000380 R11: 0000000000000246 R12: 00007fffbebe7950
>  [ 1040.204944] R13: 0000000000000007 R14: 0000000000000001 R15: 00007fffbebe7870
>  [ 1040.206686] ================================================================================
> 
> Fixes: 398c2b05bbee ("linux/dim: Add completions count to dim_sample")
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  lib/dim/dim.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/dim/dim.c b/lib/dim/dim.c
> index 439d641ec796..38045d6d0538 100644
> +++ b/lib/dim/dim.c
> @@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
>  					delta_us);
>  	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
>  	if (curr_stats->epms != 0)
> -		curr_stats->cpe_ratio =
> -				(curr_stats->cpms * 100) / curr_stats->epms;
> +		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
> +			curr_stats->cpms * 100, curr_stats->epms);

This will still potentially overfow the 'int' for cpe_ratio if epms <
100 ?

Jason

^ permalink raw reply

* Re: [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Leon Romanovsky @ 2019-07-11 15:47 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Doug Ledford, RDMA mailing list, Yamin Friedman
In-Reply-To: <20190711154324.GK25821@mellanox.com>

On Thu, Jul 11, 2019 at 03:43:28PM +0000, Jason Gunthorpe wrote:
> On Thu, Jul 11, 2019 at 06:31:18PM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > Multiply by 100 can potentially overflow cpms value and will produce
> > incorrect wrong ratio statistics. Update code to use built-in division
> > macro, so it will fix the following UBSAN warning.
> >
> >  [ 1040.120129] ================================================================================
> >  [ 1040.127124] UBSAN: Undefined behaviour in lib/dim/dim.c:78:23
> >  [ 1040.130118] signed integer overflow:
> >  [ 1040.131643] 134718714 * 100 cannot be represented in type 'int'
> >  [ 1040.134374] CPU: 0 PID: 22846 Comm: iperf3 Not tainted 5.2.0-rc6-for-upstream-dbg-2019-06-29_03-18-13-29 #1
> >  [ 1040.139068] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> >  [ 1040.144469] Call Trace:
> >  [ 1040.145897]  <IRQ>
> >  [ 1040.147366]  dump_stack+0x9a/0xeb
> >  [ 1040.149061]  ubsan_epilogue+0x9/0x7c
> >  [ 1040.150462]  handle_overflow+0x16d/0x198
> >  [ 1040.151911]  ? __ubsan_handle_negate_overflow+0x15c/0x15c
> >  [ 1040.153679]  ? sk_free+0x15/0x30
> >  [ 1040.155011]  ? kvm_clock_read+0x14/0x30
> >  [ 1040.156433]  ? kvm_sched_clock_read+0x5/0x10
> >  [ 1040.157952]  ? sched_clock+0x5/0x10
> >  [ 1040.159318]  ? sched_clock_cpu+0x18/0x260
> >  [ 1040.160801]  dim_calc_stats+0x4a1/0x4c0
> >  [ 1040.162274]  net_dim+0x147/0x920
> >  [ 1040.163592]  ? net_dim_stats_compare+0x330/0x330
> >  [ 1040.165283]  mlx5e_napi_poll+0x410/0x1030 [mlx5_core]
> >  [ 1040.166876]  ? lock_stats+0xd41/0x1740
> >  [ 1040.168266]  ? mlx5e_trigger_irq+0x550/0x550 [mlx5_core]
> >  [ 1040.169918]  ? __module_text_address+0x13/0x140
> >  [ 1040.171409]  ? lock_stats+0xd41/0x1740
> >  [ 1040.172757]  ? net_rx_action+0x262/0xda0
> >  [ 1040.174156]  net_rx_action+0x421/0xda0
> >  [ 1040.175519]  ? napi_complete_done+0x370/0x370
> >  [ 1040.176979]  ? kvm_clock_read+0x14/0x30
> >  [ 1040.178316]  ? kvm_sched_clock_read+0x5/0x10
> >  [ 1040.179690]  ? sched_clock+0x5/0x10
> >  [ 1040.180920]  ? sched_clock_cpu+0x18/0x260
> >  [ 1040.182286]  __do_softirq+0x287/0xb4e
> >  [ 1040.183581]  ? irqtime_account_irq+0x1d5/0x3b0
> >  [ 1040.184998]  irq_exit+0x17d/0x1d0
> >  [ 1040.186212]  do_IRQ+0x129/0x220
> >  [ 1040.187412]  common_interrupt+0xf/0xf
> >  [ 1040.188673]  </IRQ>
> >  [ 1040.189685] RIP: 0033:0x7f092c41a07a
> >  [ 1040.190884] Code: 45 31 f6 e9 8a 00 00 00 0f 1f 84 00 00 00 00 00 48
> > 89 df ff 93 88 01 00 00 85 c0 0f 88 c7 00 00 00 48 98 48 01 85 88 02 00
> > 00 <48> 8b 85 c8 02 00 00 48 83 85 90 02 00 00 01 48 83 78 10 00 74 0b
> >  [ 1040.195584] RSP: 002b:00007fffbebe7870 EFLAGS: 00000206 ORIG_RAX: ffffffffffffffd7
> >  [ 1040.197933] RAX: 0000000000020000 RBX: 0000000000e239b0 RCX: 000000000006b280
> >  [ 1040.199740] RDX: 0000000000020000 RSI: 00007f092c805000 RDI: 0000000000000007
> >  [ 1040.201525] RBP: 0000000000e21260 R08: 0000000000000000 R09: 00007fffbebfb0a0
> >  [ 1040.203237] R10: 0000000000000380 R11: 0000000000000246 R12: 00007fffbebe7950
> >  [ 1040.204944] R13: 0000000000000007 R14: 0000000000000001 R15: 00007fffbebe7870
> >  [ 1040.206686] ================================================================================
> >
> > Fixes: 398c2b05bbee ("linux/dim: Add completions count to dim_sample")
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  lib/dim/dim.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/dim/dim.c b/lib/dim/dim.c
> > index 439d641ec796..38045d6d0538 100644
> > +++ b/lib/dim/dim.c
> > @@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
> >  					delta_us);
> >  	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
> >  	if (curr_stats->epms != 0)
> > -		curr_stats->cpe_ratio =
> > -				(curr_stats->cpms * 100) / curr_stats->epms;
> > +		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
> > +			curr_stats->cpms * 100, curr_stats->epms);
>
> This will still potentially overfow the 'int' for cpe_ratio if epms <
> 100 ?

I assumed that assignment to "unsigned long long" will do the trick.
https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L94

Thanks

>
> Jason

^ permalink raw reply

* Re: [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Jason Gunthorpe @ 2019-07-11 16:11 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Doug Ledford, RDMA mailing list, Yamin Friedman
In-Reply-To: <20190711154734.GI23598@mtr-leonro.mtl.com>

On Thu, Jul 11, 2019 at 06:47:34PM +0300, Leon Romanovsky wrote:
> > > diff --git a/lib/dim/dim.c b/lib/dim/dim.c
> > > index 439d641ec796..38045d6d0538 100644
> > > +++ b/lib/dim/dim.c
> > > @@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
> > >  					delta_us);
> > >  	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
> > >  	if (curr_stats->epms != 0)
> > > -		curr_stats->cpe_ratio =
> > > -				(curr_stats->cpms * 100) / curr_stats->epms;
> > > +		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
> > > +			curr_stats->cpms * 100, curr_stats->epms);
> >
> > This will still potentially overfow the 'int' for cpe_ratio if epms <
> > 100 ?
> 
> I assumed that assignment to "unsigned long long" will do the trick.
> https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L94

That only protects the multiply, the result of DIV_ROUND_DOWN_ULL is
casted to int.

Jason

^ permalink raw reply

* Re: regression: nvme rdma with bnxt_re0 broken
From: Sagi Grimberg @ 2019-07-11 16:13 UTC (permalink / raw)
  To: Yi Zhang, linux-nvme; +Cc: danielj, parav, linux-rdma@vger.kernel.org
In-Reply-To: <619411460.27128070.1562838433020.JavaMail.zimbra@redhat.com>

CC linux-rdma

On 7/11/19 2:47 AM, Yi Zhang wrote:
> Hello
> 
> 'nvme connect' failed when use bnxt_re0 on latest upstream build[1], by bisecting I found it was introduced from v5.2.0-rc1 with [2], it works after I revert it.
> Let me know if you need more info, thanks.
> 
> [1]
> [root@rdma-perf-07 ~]$ nvme connect -t rdma -a 172.31.40.125 -s 4420 -n testnqn
> Failed to write to /dev/nvme-fabrics: Bad address
> 
> [root@rdma-perf-07 ~]$ dmesg
> [  476.320742] bnxt_en 0000:19:00.0: QPLIB: cmdq[0x4b9]=0x15 status 0x5
> [  476.327103] infiniband bnxt_re0: Failed to allocate HW AH
> [  476.332525] nvme nvme2: rdma_connect failed (-14).
> [  476.343552] nvme nvme2: rdma connection establishment failed (-14)
> 
> [root@rdma-perf-07 ~]$ lspci  | grep -i Broadcom
> 01:00.0 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5720 2-port Gigabit Ethernet PCIe
> 01:00.1 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5720 2-port Gigabit Ethernet PCIe
> 18:00.0 RAID bus controller: Broadcom / LSI MegaRAID SAS-3 3008 [Fury] (rev 02)
> 19:00.0 Ethernet controller: Broadcom Inc. and subsidiaries BCM57412 NetXtreme-E 10Gb RDMA Ethernet Controller (rev 01)
> 19:00.1 Ethernet controller: Broadcom Inc. and subsidiaries BCM57412 NetXtreme-E 10Gb RDMA Ethernet Controller (rev 01)
> 
> 
> [2]
> commit 823b23da71132b80d9f41ab667c68b112455f3b6
> Author: Parav Pandit <parav@mellanox.com>
> Date:   Wed Apr 10 11:23:03 2019 +0300
> 
>      IB/core: Allow vlan link local address based RoCE GIDs
>      
>      IPv6 link local address for a VLAN netdevice has nothing to do with its
>      resemblance with the default GID, because VLAN link local GID is in
>      different layer 2 domain.
>      
>      Now that RoCE MAD packet processing and route resolution consider the
>      right GID index, there is no need for an unnecessary check which prevents
>      the addition of vlan based IPv6 link local GIDs.
>      
>      Signed-off-by: Parav Pandit <parav@mellanox.com>
>      Reviewed-by: Daniel Jurgens <danielj@mellanox.com>
>      Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>      Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> 
> 
> 
> Best Regards,
>    Yi Zhang
> 
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
> 

^ permalink raw reply

* RE: regression: nvme rdma with bnxt_re0 broken
From: Parav Pandit @ 2019-07-11 16:18 UTC (permalink / raw)
  To: Yi Zhang, linux-nvme@lists.infradead.org
  Cc: Daniel Jurgens, linux-rdma@vger.kernel.org, Devesh Sharma,
	selvin.xavier@broadcom.com
In-Reply-To: <AM0PR05MB48664657022ECA8526E3C967D1F30@AM0PR05MB4866.eurprd05.prod.outlook.com>

Sagi,

This is better one to cc to linux-rdma.

+ Devesh, Selvin.

> -----Original Message-----
> From: Parav Pandit
> Sent: Thursday, July 11, 2019 6:25 PM
> To: Yi Zhang <yi.zhang@redhat.com>; linux-nvme@lists.infradead.org
> Cc: Daniel Jurgens <danielj@mellanox.com>
> Subject: RE: regression: nvme rdma with bnxt_re0 broken
> 
> Hi Yi Zhang,
> 
> > -----Original Message-----
> > From: Yi Zhang <yi.zhang@redhat.com>
> > Sent: Thursday, July 11, 2019 3:17 PM
> > To: linux-nvme@lists.infradead.org
> > Cc: Daniel Jurgens <danielj@mellanox.com>; Parav Pandit
> > <parav@mellanox.com>
> > Subject: regression: nvme rdma with bnxt_re0 broken
> >
> > Hello
> >
> > 'nvme connect' failed when use bnxt_re0 on latest upstream build[1],
> > by bisecting I found it was introduced from v5.2.0-rc1 with [2], it
> > works after I revert it.
> > Let me know if you need more info, thanks.
> >
> > [1]
> > [root@rdma-perf-07 ~]$ nvme connect -t rdma -a 172.31.40.125 -s 4420
> > -n testnqn Failed to write to /dev/nvme-fabrics: Bad address
> >
> > [root@rdma-perf-07 ~]$ dmesg
> > [  476.320742] bnxt_en 0000:19:00.0: QPLIB: cmdq[0x4b9]=0x15 status
> > 0x5 [ 476.327103] infiniband bnxt_re0: Failed to allocate HW AH [
> > 476.332525] nvme nvme2: rdma_connect failed (-14).
> > [  476.343552] nvme nvme2: rdma connection establishment failed (-14)
> >
> > [root@rdma-perf-07 ~]$ lspci  | grep -i Broadcom
> > 01:00.0 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme
> > BCM5720 2-port Gigabit Ethernet PCIe
> > 01:00.1 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme
> > BCM5720 2-port Gigabit Ethernet PCIe
> > 18:00.0 RAID bus controller: Broadcom / LSI MegaRAID SAS-3 3008 [Fury]
> > (rev
> > 02)
> > 19:00.0 Ethernet controller: Broadcom Inc. and subsidiaries BCM57412
> > NetXtreme-E 10Gb RDMA Ethernet Controller (rev 01)
> > 19:00.1 Ethernet controller: Broadcom Inc. and subsidiaries BCM57412
> > NetXtreme-E 10Gb RDMA Ethernet Controller (rev 01)
> >
> >
> > [2]
> > commit 823b23da71132b80d9f41ab667c68b112455f3b6
> > Author: Parav Pandit <parav@mellanox.com>
> > Date:   Wed Apr 10 11:23:03 2019 +0300
> >
> >     IB/core: Allow vlan link local address based RoCE GIDs
> >
> >     IPv6 link local address for a VLAN netdevice has nothing to do with its
> >     resemblance with the default GID, because VLAN link local GID is in
> >     different layer 2 domain.
> >
> >     Now that RoCE MAD packet processing and route resolution consider the
> >     right GID index, there is no need for an unnecessary check which prevents
> >     the addition of vlan based IPv6 link local GIDs.
> >
> >     Signed-off-by: Parav Pandit <parav@mellanox.com>
> >     Reviewed-by: Daniel Jurgens <danielj@mellanox.com>
> >     Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >     Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> >
> >
> >
> > Best Regards,
> >   Yi Zhang
> >
> 
> I need some more information from you to debug this issue as I don’t have the
> hw.
> The highlighted patch added support for IPv6 link local address for vlan. I am
> unsure how this can affect IPv4 AH creation for which there is failure.
> 
> 1. Before you assign the IP address to the netdevice, Please do, echo -n
> "module ib_core +p" > /sys/kernel/debug/dynamic_debug/control
> 
> Please share below output before doing nvme connect.
> 2. Output of script [1]
> $ show_gids script
> If getting this script is problematic, share the output of,
> 
> $ cat /sys/class/infiniband/bnxt_re0/ports/1/gids/*
> $ cat /sys/class/infiniband/bnxt_re0/ports/1/gid_attrs/ndevs/*
> $ ip link show
> $ip addr show
> $ dmesg
> 
> [1] https://community.mellanox.com/s/article/understanding-show-gids-
> script#jive_content_id_The_Script
> 
> I suspect that driver's assumption about GID indices might have gone wrong
> here in drivers/infiniband/hw/bnxt_re/ib_verbs.c.
> Lets see about results to confirm that.

^ permalink raw reply

* Re: [PATCH] RDMA/siw: Mark expected switch fall-throughs
From: Bernard Metzler @ 2019-07-11 16:28 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel
In-Reply-To: <20190711161218.GA4989@embeddedor>

-----"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: -----

>To: "Bernard Metzler" <bmt@zurich.ibm.com>, "Doug Ledford"
><dledford@redhat.com>, "Jason Gunthorpe" <jgg@ziepe.ca>
>From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
>Date: 07/11/2019 06:12PM
>Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
>"Gustavo A. R. Silva" <gustavo@embeddedor.com>
>Subject: [EXTERNAL] [PATCH] RDMA/siw: Mark expected switch
>fall-throughs
>
>In preparation to enabling -Wimplicit-fallthrough, mark switch
>cases where we are expecting to fall through.
>
>This patch fixes the following warnings:
>
>drivers/infiniband/sw/siw/siw_qp_rx.c: In function
>‘siw_rdmap_complete’:
>drivers/infiniband/sw/siw/siw_qp_rx.c:1214:18: warning: this
>statement may fall through [-Wimplicit-fallthrough=]
>   wqe->rqe.flags |= SIW_WQE_SOLICITED;
>   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
>drivers/infiniband/sw/siw/siw_qp_rx.c:1215:2: note: here
>  case RDMAP_SEND:
>  ^~~~
>
>drivers/infiniband/sw/siw/siw_qp_tx.c: In function
>‘siw_qp_sq_process’:
>drivers/infiniband/sw/siw/siw_qp_tx.c:1044:4: warning: this statement
>may fall through [-Wimplicit-fallthrough=]
>    siw_wqe_put_mem(wqe, tx_type);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>drivers/infiniband/sw/siw/siw_qp_tx.c:1045:3: note: here
>   case SIW_OP_INVAL_STAG:
>   ^~~~
>drivers/infiniband/sw/siw/siw_qp_tx.c:1128:4: warning: this statement
>may fall through [-Wimplicit-fallthrough=]
>    siw_wqe_put_mem(wqe, tx_type);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>drivers/infiniband/sw/siw/siw_qp_tx.c:1129:3: note: here
>   case SIW_OP_INVAL_STAG:
>   ^~~~
>
>Warning level 3 was used: -Wimplicit-fallthrough=3
>
>This patch is part of the ongoing efforts to enable
>-Wimplicit-fallthrough.
>
>Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>---
>
>NOTE: -Wimplicit-fallthrough will be enabled globally in v5.3. So, I
>      suggest you to take this patch for 5.3-rc1.
>
> drivers/infiniband/sw/siw/siw_qp_rx.c | 2 ++
> drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ++++
> 2 files changed, 6 insertions(+)
>
>diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c
>b/drivers/infiniband/sw/siw/siw_qp_rx.c
>index 682a290bc11e..f87657a11657 100644
>--- a/drivers/infiniband/sw/siw/siw_qp_rx.c
>+++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
>@@ -1212,6 +1212,8 @@ static int siw_rdmap_complete(struct siw_qp
>*qp, int error)
> 	case RDMAP_SEND_SE:
> 	case RDMAP_SEND_SE_INVAL:
> 		wqe->rqe.flags |= SIW_WQE_SOLICITED;
>+		/* Fall through */
>+
> 	case RDMAP_SEND:
> 	case RDMAP_SEND_INVAL:
> 		if (wqe->wr_status == SIW_WR_IDLE)
>diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c
>b/drivers/infiniband/sw/siw/siw_qp_tx.c
>index f0d949e2e318..43020d2040fc 100644
>--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
>+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
>@@ -1042,6 +1042,8 @@ int siw_qp_sq_process(struct siw_qp *qp)
> 		case SIW_OP_SEND_REMOTE_INV:
> 		case SIW_OP_WRITE:
> 			siw_wqe_put_mem(wqe, tx_type);
>+			/* Fall through */
>+
> 		case SIW_OP_INVAL_STAG:
> 		case SIW_OP_REG_MR:
> 			if (tx_flags(wqe) & SIW_WQE_SIGNALLED)
>@@ -1126,6 +1128,8 @@ int siw_qp_sq_process(struct siw_qp *qp)
> 		case SIW_OP_READ:
> 		case SIW_OP_READ_LOCAL_INV:
> 			siw_wqe_put_mem(wqe, tx_type);
>+			/* Fall through */
>+
> 		case SIW_OP_INVAL_STAG:
> 		case SIW_OP_REG_MR:
> 			siw_sqe_complete(qp, &wqe->sqe, wqe->bytes,
>-- 
>2.21.0
>
>
Thanks Gustavo. Didn't know that becomes mandatory. Had it here and there
only, where I thought it is not obvious...but better to keep the rules
simple.

Thanks,
Bernard.


^ permalink raw reply

* [PATCH] RDMA/siw: Mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2019-07-11 16:12 UTC (permalink / raw)
  To: Bernard Metzler, Doug Ledford, Jason Gunthorpe
  Cc: linux-rdma, linux-kernel, Gustavo A. R. Silva

In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

drivers/infiniband/sw/siw/siw_qp_rx.c: In function ‘siw_rdmap_complete’:
drivers/infiniband/sw/siw/siw_qp_rx.c:1214:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
   wqe->rqe.flags |= SIW_WQE_SOLICITED;
   ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
drivers/infiniband/sw/siw/siw_qp_rx.c:1215:2: note: here
  case RDMAP_SEND:
  ^~~~

drivers/infiniband/sw/siw/siw_qp_tx.c: In function ‘siw_qp_sq_process’:
drivers/infiniband/sw/siw/siw_qp_tx.c:1044:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
    siw_wqe_put_mem(wqe, tx_type);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/infiniband/sw/siw/siw_qp_tx.c:1045:3: note: here
   case SIW_OP_INVAL_STAG:
   ^~~~
drivers/infiniband/sw/siw/siw_qp_tx.c:1128:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
    siw_wqe_put_mem(wqe, tx_type);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/infiniband/sw/siw/siw_qp_tx.c:1129:3: note: here
   case SIW_OP_INVAL_STAG:
   ^~~~

Warning level 3 was used: -Wimplicit-fallthrough=3

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---

NOTE: -Wimplicit-fallthrough will be enabled globally in v5.3. So, I
      suggest you to take this patch for 5.3-rc1.

 drivers/infiniband/sw/siw/siw_qp_rx.c | 2 ++
 drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/infiniband/sw/siw/siw_qp_rx.c b/drivers/infiniband/sw/siw/siw_qp_rx.c
index 682a290bc11e..f87657a11657 100644
--- a/drivers/infiniband/sw/siw/siw_qp_rx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_rx.c
@@ -1212,6 +1212,8 @@ static int siw_rdmap_complete(struct siw_qp *qp, int error)
 	case RDMAP_SEND_SE:
 	case RDMAP_SEND_SE_INVAL:
 		wqe->rqe.flags |= SIW_WQE_SOLICITED;
+		/* Fall through */
+
 	case RDMAP_SEND:
 	case RDMAP_SEND_INVAL:
 		if (wqe->wr_status == SIW_WR_IDLE)
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
index f0d949e2e318..43020d2040fc 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -1042,6 +1042,8 @@ int siw_qp_sq_process(struct siw_qp *qp)
 		case SIW_OP_SEND_REMOTE_INV:
 		case SIW_OP_WRITE:
 			siw_wqe_put_mem(wqe, tx_type);
+			/* Fall through */
+
 		case SIW_OP_INVAL_STAG:
 		case SIW_OP_REG_MR:
 			if (tx_flags(wqe) & SIW_WQE_SIGNALLED)
@@ -1126,6 +1128,8 @@ int siw_qp_sq_process(struct siw_qp *qp)
 		case SIW_OP_READ:
 		case SIW_OP_READ_LOCAL_INV:
 			siw_wqe_put_mem(wqe, tx_type);
+			/* Fall through */
+
 		case SIW_OP_INVAL_STAG:
 		case SIW_OP_REG_MR:
 			siw_sqe_complete(qp, &wqe->sqe, wqe->bytes,
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH] rdma/siw: Use proper enumerated type in map_cqe_status
From: Nick Desaulniers @ 2019-07-11 17:16 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Nathan Chancellor, Bernard Metzler, Doug Ledford, linux-rdma,
	LKML, clang-built-linux
In-Reply-To: <20190711133915.GA25807@ziepe.ca>

On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > Maybe time to start plumbing Clang into your test flow until it can get
> > intergrated with more CI setups? :) It can catch some pretty dodgy
> > behavior that GCC doesn't:
>
> I keep asking how to use clang to build the kernel and last I was told
> it still wasn't ready..
>
> Is it ready now? Is there some flow that will compile with clang
> warning free, on any arch? (at least the portion of the kernel I check)

$ make CC=clang ...

Let us know if you find something we haven't already.
https://clangbuiltlinux.github.io/
https://github.com/ClangBuiltLinux/linux/issues
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: [PATCH] rdma/siw: Use proper enumerated type in map_cqe_status
From: Jason Gunthorpe @ 2019-07-11 17:18 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Bernard Metzler, Doug Ledford, linux-rdma,
	LKML, clang-built-linux
In-Reply-To: <CAKwvOdnHz3uH4ZM20LGQJ3FYhLQQUYn4Lg0B-YMr7Y1L66TAsA@mail.gmail.com>

On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > Maybe time to start plumbing Clang into your test flow until it can get
> > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > behavior that GCC doesn't:
> >
> > I keep asking how to use clang to build the kernel and last I was told
> > it still wasn't ready..
> >
> > Is it ready now? Is there some flow that will compile with clang
> > warning free, on any arch? (at least the portion of the kernel I check)
> 
> $ make CC=clang ...
> 
> Let us know if you find something we haven't already.
> https://clangbuiltlinux.github.io/
> https://github.com/ClangBuiltLinux/linux/issues

What clang version?

Jason

^ permalink raw reply

* Re: [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Leon Romanovsky @ 2019-07-11 17:19 UTC (permalink / raw)
  To: Jason Gunthorpe; +Cc: Doug Ledford, RDMA mailing list, Yamin Friedman
In-Reply-To: <20190711161103.GL25821@mellanox.com>

On Thu, Jul 11, 2019 at 04:11:07PM +0000, Jason Gunthorpe wrote:
> On Thu, Jul 11, 2019 at 06:47:34PM +0300, Leon Romanovsky wrote:
> > > > diff --git a/lib/dim/dim.c b/lib/dim/dim.c
> > > > index 439d641ec796..38045d6d0538 100644
> > > > +++ b/lib/dim/dim.c
> > > > @@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
> > > >  					delta_us);
> > > >  	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
> > > >  	if (curr_stats->epms != 0)
> > > > -		curr_stats->cpe_ratio =
> > > > -				(curr_stats->cpms * 100) / curr_stats->epms;
> > > > +		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
> > > > +			curr_stats->cpms * 100, curr_stats->epms);
> > >
> > > This will still potentially overfow the 'int' for cpe_ratio if epms <
> > > 100 ?
> >
> > I assumed that assignment to "unsigned long long" will do the trick.
> > https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L94
>
> That only protects the multiply, the result of DIV_ROUND_DOWN_ULL is
> casted to int.

It is ok, the result is "int" and it will be small, 100 in multiply
represents percentage.

Thanks

>
> Jason

^ permalink raw reply

* Re: [PATCH] rdma/siw: Use proper enumerated type in map_cqe_status
From: Nathan Chancellor @ 2019-07-11 17:30 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Nick Desaulniers, Bernard Metzler, Doug Ledford, linux-rdma, LKML,
	clang-built-linux
In-Reply-To: <20190711171808.GF25807@ziepe.ca>

On Thu, Jul 11, 2019 at 02:18:08PM -0300, Jason Gunthorpe wrote:
> On Thu, Jul 11, 2019 at 10:16:44AM -0700, Nick Desaulniers wrote:
> > On Thu, Jul 11, 2019 at 6:39 AM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >
> > > On Thu, Jul 11, 2019 at 01:14:34AM -0700, Nathan Chancellor wrote:
> > > > Maybe time to start plumbing Clang into your test flow until it can get
> > > > intergrated with more CI setups? :) It can catch some pretty dodgy
> > > > behavior that GCC doesn't:
> > >
> > > I keep asking how to use clang to build the kernel and last I was told
> > > it still wasn't ready..
> > >
> > > Is it ready now? Is there some flow that will compile with clang
> > > warning free, on any arch? (at least the portion of the kernel I check)
> > 
> > $ make CC=clang ...
> > 
> > Let us know if you find something we haven't already.
> > https://clangbuiltlinux.github.io/
> > https://github.com/ClangBuiltLinux/linux/issues
> 
> What clang version?
> 
> Jason

You'll need clang-9 for x86 because of the asm-goto requirement (or a
selective set of reverts for clang-8) but everything else should be
good with clang-8:

https://travis-ci.com/ClangBuiltLinux/continuous-integration/builds/118745131

We wrote a Python script that builds an LLVM 9 toolchain suitable for
kernel development that is self contained (doesn't install anything to
your system):

https://github.com/ClangBuiltLinux/tc-build

Let me know if there are any issues with it if you give it a go, I've
already fixed one from Thomas Gleixner:

https://lore.kernel.org/lkml/alpine.DEB.2.21.1906262140570.32342@nanos.tec.linutronix.de/

Cheers,
Nathan

^ permalink raw reply

* Re: [PATCH rdma-next] lib/dim: Prevent overflow in calculation of ratio statistics
From: Jason Gunthorpe @ 2019-07-11 17:31 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Doug Ledford, RDMA mailing list, Yamin Friedman
In-Reply-To: <20190711171922.GJ23598@mtr-leonro.mtl.com>

On Thu, Jul 11, 2019 at 08:19:22PM +0300, Leon Romanovsky wrote:
> On Thu, Jul 11, 2019 at 04:11:07PM +0000, Jason Gunthorpe wrote:
> > On Thu, Jul 11, 2019 at 06:47:34PM +0300, Leon Romanovsky wrote:
> > > > > diff --git a/lib/dim/dim.c b/lib/dim/dim.c
> > > > > index 439d641ec796..38045d6d0538 100644
> > > > > +++ b/lib/dim/dim.c
> > > > > @@ -74,8 +74,8 @@ void dim_calc_stats(struct dim_sample *start, struct dim_sample *end,
> > > > >  					delta_us);
> > > > >  	curr_stats->cpms = DIV_ROUND_UP(ncomps * USEC_PER_MSEC, delta_us);
> > > > >  	if (curr_stats->epms != 0)
> > > > > -		curr_stats->cpe_ratio =
> > > > > -				(curr_stats->cpms * 100) / curr_stats->epms;
> > > > > +		curr_stats->cpe_ratio = DIV_ROUND_DOWN_ULL(
> > > > > +			curr_stats->cpms * 100, curr_stats->epms);
> > > >
> > > > This will still potentially overfow the 'int' for cpe_ratio if epms <
> > > > 100 ?
> > >
> > > I assumed that assignment to "unsigned long long" will do the trick.
> > > https://elixir.bootlin.com/linux/latest/source/include/linux/kernel.h#L94
> >
> > That only protects the multiply, the result of DIV_ROUND_DOWN_ULL is
> > casted to int.
> 
> It is ok, the result is "int" and it will be small, 100 in multiply
> represents percentage.

Percentage would be divide by 100..

Like I said it will overflow if epms < 100 ...

Jason

^ permalink raw reply

* Re: [PATCH] RDMA/siw: Mark expected switch fall-throughs
From: Jason Gunthorpe @ 2019-07-11 18:05 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Bernard Metzler, Doug Ledford, linux-rdma, linux-kernel
In-Reply-To: <20190711161218.GA4989@embeddedor>

On Thu, Jul 11, 2019 at 11:12:18AM -0500, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch
> cases where we are expecting to fall through.
> 
> This patch fixes the following warnings:
> 
> drivers/infiniband/sw/siw/siw_qp_rx.c: In function ‘siw_rdmap_complete’:
> drivers/infiniband/sw/siw/siw_qp_rx.c:1214:18: warning: this statement may fall through [-Wimplicit-fallthrough=]
>    wqe->rqe.flags |= SIW_WQE_SOLICITED;
>    ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
> drivers/infiniband/sw/siw/siw_qp_rx.c:1215:2: note: here
>   case RDMAP_SEND:
>   ^~~~
> 
> drivers/infiniband/sw/siw/siw_qp_tx.c: In function ‘siw_qp_sq_process’:
> drivers/infiniband/sw/siw/siw_qp_tx.c:1044:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     siw_wqe_put_mem(wqe, tx_type);
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/infiniband/sw/siw/siw_qp_tx.c:1045:3: note: here
>    case SIW_OP_INVAL_STAG:
>    ^~~~
> drivers/infiniband/sw/siw/siw_qp_tx.c:1128:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     siw_wqe_put_mem(wqe, tx_type);
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/infiniband/sw/siw/siw_qp_tx.c:1129:3: note: here
>    case SIW_OP_INVAL_STAG:
>    ^~~~
> 
> Warning level 3 was used: -Wimplicit-fallthrough=3
> 
> This patch is part of the ongoing efforts to enable
> -Wimplicit-fallthrough.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> 
> NOTE: -Wimplicit-fallthrough will be enabled globally in v5.3. So, I
>       suggest you to take this patch for 5.3-rc1.

Okay, I queued this for the current merge window then

Jason

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox