* Re: [PATCH rdma-core 0/4] rdma-core infrastructure
From: Jarod Wilson @ 2016-10-17 15:51 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161015092619.GJ9282-2ukJVAZIZ/Y@public.gmane.org>
On Sat, Oct 15, 2016 at 12:26:19PM +0300, Leon Romanovsky wrote:
> On Fri, Oct 14, 2016 at 03:21:32PM -0400, Jarod Wilson wrote:
> > This is a set of patches to make some minor changes to the stock spec file,
> > add a bunch of userspace glue that Red Hat has been shipping on it's own in
> > Red Hat Enterprise Linux and Fedora for a while now, and a revised split
> > package spec file that we're likely to use in at least RHEL, to preserve the
> > current packaging split.
>
> Jarod,
>
> I'm far a way from packaging, so I have very general questions.
> 1. You mentioned FC, but used glue/redhat/. Is it applicable for Centos?
Red Hat is the driving force behind Red Hat Enterprise Linux, CentOS (or
any other clone) and Fedora, so it would generally be applicable for all
of them, similar to how the 'debian' stuff is generally applicable to
derivatives like ubuntu as well.
> 2. Do FC and Centos need similar files in structure?
For the most part, they should be identical.
> 3. Does it make sense to rename glue/ to be packages/ and move debian there?
That, or as Jason has suggested, just have a top-level redhat directory,
same as the debian one. Don't really care much what it's called, just as
long as we can get (at least most of) this stuff into the tree, rather
than having it lingering in our own special package.
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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-core 1/4] rpm spec: install documentation too
From: Jarod Wilson @ 2016-10-17 15:53 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161014225204.GB16509-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Fri, Oct 14, 2016 at 04:52:04PM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 14, 2016 at 03:21:33PM -0400, Jarod Wilson wrote:
> > At least in the Red Hat world, we like to install the documentation files
> > provided in a given source tree.
>
> I think other distros would like to do this too, can you use this
> patch instead?
Ah. Yeah. My cmake-fu is ... lacking. That does look like a better way to
go. I'll pull this into my working tree and play with it.
> From a23838ccc05574b1975741770abcbd66f7587293 Mon Sep 17 00:00:00 2001
> From: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Date: Fri, 14 Oct 2016 15:21:33 -0400
> Subject: [PATCH] Install end user focused documentation files
>
> We recommend that all distros ship these.
>
> Suggested-by: Jarod Wilson <jarod-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
> CMakeLists.txt | 3 ++-
> Documentation/CMakeLists.txt | 8 ++++++++
> debian/ibacm.install | 1 +
> debian/ibverbs-providers.install | 1 +
> debian/libibcm1.install | 1 +
> debian/libibverbs1.install | 1 +
> debian/librdmacm1.install | 1 +
> debian/srptools.install | 1 +
> rdma-core.spec | 4 +++-
> 9 files changed, 19 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/CMakeLists.txt
...
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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 03/12] IB/hfi1: Fix an Oops on pci device force remove
From: Jason Gunthorpe @ 2016-10-17 16:07 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, Dean Luick, Ira Weiny,
Tadeusz Struk
In-Reply-To: <20161017111918.7934.72325.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org>
On Mon, Oct 17, 2016 at 04:19:19AM -0700, Dennis Dalessandro wrote:
> @@ -183,6 +183,7 @@ static int hfi1_file_open(struct inode *inode, struct file *fp)
> fd->rec_cpu_num = -1; /* no cpu affinity by default */
> fd->mm = current->mm;
> atomic_inc(&fd->mm->mm_count);
> + atomic_inc(&dd->user_refcount);
This doesn't look like enough locking on acquire.. What prevents the
driver from already having been unloaded at this point?
> +static void wait_for_clients(struct hfi1_devdata *dd)
> +{
> + /*
> + * Remove the device init value and wait for active
> + * users to finish if there are any.
> + */
> + if (atomic_sub_return(1, &dd->user_refcount))
> + wait_for_completion(&dd->user_comp);
> +}
Shouldn't this be a while loop to avoid races?
.. and shouldn't the inc 'latch off' once the removal process starts?
I think you need to study how kernfs_drain is implemented and
basically copy that approach, including the locking.
FWIW, it is much easier and probably long term better to arrange
things so the FD can remain open but just return ENODEV to all calls.
Eg the srcu based approach in uverbs.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 0/4] rdma-core infrastructure
From: Jason Gunthorpe @ 2016-10-17 16:11 UTC (permalink / raw)
To: Jarod Wilson; +Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161017155159.GG14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Oct 17, 2016 at 11:51:59AM -0400, Jarod Wilson wrote:
> > 3. Does it make sense to rename glue/ to be packages/ and move debian there?
>
> That, or as Jason has suggested, just have a top-level redhat directory,
> same as the debian one. Don't really care much what it's called, just as
> long as we can get (at least most of) this stuff into the tree, rather
> than having it lingering in our own special package.
The debian/ directory has to be top level for debian tools, so I'm
inclined to just follow the pattern and go with redhat/.
I think it is no problem to take what you have right away, but lets
try to move as much as is trivial/sensible into common places.
Maybe we can tackle the larger questions (eg systemd) later..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jarod Wilson @ 2016-10-17 16:22 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161014231934.GC16509-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Fri, Oct 14, 2016 at 05:19:34PM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 14, 2016 at 03:21:34PM -0400, Jarod Wilson wrote:
> > Red Hat has been shipping an "rdma" package, as well as it's own systemd
> > unit files for some daemons for a while now, in both Fedora and Red Hat
> > Enterprise Linux. Some of these are fairly RH-specific, but might be of
> > use to others, so we'd like to move them into the upstream source tree.
>
> We have a directory called 'debian', so lets just have one called
> 'redhat'..
Worksforme. As mentioned in reply to Leon, I really don't care what the
directory is called, just that we get this stuff out there, instead of
being in our own custom glue package. I do think maybe Leon's suggestion
of having a directory to put a bunch of distro-specific directories under
would be good for cleanliness, but I also seem to recall that debian
actually looks for a directory in the root of the tarball, so it may need
to stay like it is.
> The below comments are my view on what we should try and move into
> common cross-distro locations..
I should have been more clear: some of this is just an initial dump of
everything we ship, and there's absolutely a desire to extract anything
from this pile that is generic and usable by all distros into a common
location. Just pressed for time, and sending it out this way was the
fastest way to get it in front of eyeballs. :)
> Common stuff should be installed via cmake
>
> > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > new file mode 100644
> > index 0000000..1cd031a
> > +++ b/glue/redhat/ibacm.service
>
> Can we just put this in ibacm/ ?
Probably.
> > +Requires=rdma.service
> > +After=rdma.service opensm.service
>
> This is the only RH specific thiing I see.. Could we standardize on
> something here and use it on all distros? rdma-available.target?
I'll defer to what Doug said on this one.
> > +++ b/glue/redhat/iwpmd.service
> > @@ -0,0 +1,12 @@
> > +[Unit]
> > +Description=Starts the IWPMD daemon
> > +Documentation=file:///usr/share/doc/iwpmd/README
>
> File does not exit? There is a man page now
>
> We already have a iwpmd/iwpmd.service that is almost identical, can you
> you just update it and drop this version?
Bah, that's a carryover from our individually packaged iwpmd, didn't look
closely enough. Can we merge a bit from ours into the 'stock' one? The
main relevant difference I see is we have ours set to load after
syslog.target as well as network.target.
> > +++ b/glue/redhat/rdma.cxgb3.sys.modprobe
> > @@ -0,0 +1 @@
> > +install cxgb3 /sbin/modprobe --ignore-install cxgb3 $CMDLINE_OPTS && /sbin/modprobe iw_cxgb3
> > diff --git a/glue/redhat/rdma.cxgb4.sys.modprobe b/glue/redhat/rdma.cxgb4.sys.modprobe
> > new file mode 100644
> > index 0000000..44163ab
> > +++ b/glue/redhat/rdma.cxgb4.sys.modprobe
> > @@ -0,0 +1 @@
> > +install cxgb4 /sbin/modprobe --ignore-install cxgb4 $CMDLINE_OPTS && /sbin/modprobe iw_cxgb4
>
> What are these for? Should they be cross distro? Why are only a few
> drivers this special?
What Doug said.
> > +++ b/glue/redhat/rdma.fixup-mtrr.awk
> > @@ -0,0 +1,160 @@
> > +# This is a simple script that checks the contents of /proc/mtrr to see if
> > +# the BIOS maker for the computer took the easy way out in terms of
> > +# specifying memory regions when there is a hole below 4GB for PCI access
> > +# and the machine has 4GB or more of RAM. When the contents of /proc/mtrr
> > +# show a 4GB mapping of write-back cached RAM, minus punch out hole(s) of
> > +# uncacheable regions (the area reserved for PCI access), then it becomes
> > +# impossible for the ib_ipath driver to set write_combining on its PIO
> > +# buffers. To correct the problem, remap the lower memory region in various
> > +# chunks up to the start of the punch out hole(s), then delete the punch out
> > +# hole(s) entirely as they aren't needed any more. That way, ib_ipath will
> > +# be able to set write_combining on its PIO memory access region.
>
> Yuk, a thousand times yuk.
>
> I thought the mtrr cleanup built into modern kernel took care of this?
>
> Really though, this needs to be fixed upstream in the kernel :|
Yeah, this is still floating around for hysterical raisins, as Doug said.
We might be able to drop this, or at least relegate it to a dark corner
with a huge warning label...
> > diff --git a/glue/redhat/rdma.kernel-init b/glue/redhat/rdma.kernel-init
> > new file mode 100644
> > index 0000000..6cb4732
> > +++ b/glue/redhat/rdma.kernel-init
>
> I wonder if this could be split into a generic 'load the modules' part
> and a distro specific part? Every distro needs systemd to load the
> extra modules because out auto-loading is broken - IMHO, and that is
> pretty complex unfortunately.
Possibly. Not sure how much you'd save by splitting it though, the added
complexity of playing connect the dots to see how things come up could be
counter-productive. Haven't looked very closely at that though.
> > +errata_58()
> > +{
> > + # Check AMD chipset issue Errata #58
> > + if test -x /sbin/lspci && test -x /sbin/setpci; then
> > + if ( /sbin/lspci -nd 1022:1100 | grep "1100" > /dev/null ) &&
> > + ( /sbin/lspci -nd 1022:7450 | grep "7450" > /dev/null ) &&
> > + ( /sbin/lspci -nd 15b3:5a46 | grep "5a46" > /dev/null ); then
> > + CURVAL=`/sbin/setpci -d 1022:1100 69`
>
> Another yuk. Why isn't this handled upstream in drivers/pci/quirks.c
> with the rest of the 8131 errata?
>
> Fortunately I expect all 8131 hardware is long since gone, that chip
> was end-of-manufacturing'd very quickly 2003ish IIRC.
More hysterical raisins.
> > +++ b/glue/redhat/rdma.service
> > @@ -0,0 +1,15 @@
> > +[Unit]
> > +Description=Initialize the iWARP/InfiniBand/RDMA stack in the kernel
> > +Documentation=file:/etc/rdma/rdma.conf
> > +RefuseManualStop=true
> > +DefaultDependencies=false
> > +Conflicts=emergency.target emergency.service
> > +Before=network.target remote-fs-pre.target
>
> This is an area we really need to cross-distro standardize - we really
> need a set of rdma-*.targets.
>
> eg
> rdma-available.target
> - RDMA hardware is available and all prep is done
> opensm (if installed) is started, etc
> Use in place of rdma.service
> rdma-detected.target
> - udev detected rdma hardware
Deferring to Doug ehre too.
> > +++ b/glue/redhat/rdma.udev-ipoib-naming.rules
> > @@ -0,0 +1,13 @@
> > +# This is a sample udev rules file that demonstrates how to get udev to
> > +# set the name of IPoIB interfaces to whatever you wish. There is a
> > +# 16 character limit on network device names though, so don't go too nuts
> > +#
> > +# Important items to note: ATTR{type}=="32" is IPoIB interfaces, and the
> > +# ATTR{address} match must start with ?* and only reference the last 8
> > +# bytes of the address or else the address might not match on any given
> > +# start of the IPoIB stack
> > +#
> > +# Note: as of rhel7, udev is case sensitive on the address field match
> > +# and all addresses need to be in lower case.
> > +#
> > +# ACTION=="add", SUBSYSTEM=="net", DRIVERS=="?*", ATTR{type}=="32", ATTR{address}=="?*00:02:c9:03:00:31:78:f2", NAME="mlx4_ib3"
>
> This should be a cross distro file.
>
> > +++ b/glue/redhat/rdma.udev-rules
> > @@ -0,0 +1,18 @@
> > +# We list all the various kernel modules that drive hardware in the
> > +# InfiniBand stack (and a few in the network stack that might not actually
> > +# be RDMA capable, but we don't know that at this time and it's safe to
> > +# enable the IB stack, so do so unilaterally) and on load of any of that
> > +# hardware, we trigger the rdma.service load in systemd
> > +
> > +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
>
> Also cross distro
Yeah, these are definitely prime candidates for being cross-distro. And
really, I was thinking maybe these should be part of the core upstream
udev/systemd rules set, rather than something we ship here.
> > +# When we detect a new verbs device is added to the system, set the node
> > +# description on that device
> > +# If rdma-ndd is installed, defer the setting of the node description to it.
> > +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
>
> Shouldn't this udev drop-in by in the rdma-ndd package?
Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
Doug said about this one...
> > +++ b/glue/redhat/srp_daemon.service
> > @@ -0,0 +1,17 @@
> > +[Unit]
> > +Description=Start or stop the daemon that attaches to SRP devices
> > +Documentation=file:///etc/rdma/rdma.conf file:///etc/srp_daemon.conf
> > +DefaultDependencies=false
> > +Conflicts=emergency.target emergency.service
> > +Requires=rdma.service
> > +Wants=opensm.service
> > +After=rdma.service opensm.service
> > +After=network.target
> > +Before=remote-fs-pre.target
>
> Also should be common, why does it reference opensm.service?
>
> > +
> > +[Service]
> > +Type=simple
> > +ExecStart=/usr/sbin/srp_daemon.sh
>
> Hurm, someday we have to make better systemd integration for these
> daemons..
More deferrals to Doug!
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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-core 0/4] rdma-core infrastructure
From: Jarod Wilson @ 2016-10-17 16:26 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Leon Romanovsky, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161017161154.GB5679-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Mon, Oct 17, 2016 at 10:11:54AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 17, 2016 at 11:51:59AM -0400, Jarod Wilson wrote:
>
> > > 3. Does it make sense to rename glue/ to be packages/ and move debian there?
> >
> > That, or as Jason has suggested, just have a top-level redhat directory,
> > same as the debian one. Don't really care much what it's called, just as
> > long as we can get (at least most of) this stuff into the tree, rather
> > than having it lingering in our own special package.
>
> The debian/ directory has to be top level for debian tools, so I'm
> inclined to just follow the pattern and go with redhat/.
Okay, good, my vague recollection of that was correct.
> I think it is no problem to take what you have right away, but lets
> try to move as much as is trivial/sensible into common places.
I'm shuffling things around locally to redhat/ and have pulled in your
Docs install change. Want me to finish cleaning up, double-check that it
all builds, and ship a v2 set?
> Maybe we can tackle the larger questions (eg systemd) later..
Some of these issues might be better addressed once more distros have had
a chance to digest the new package and see what they still need. Entirely
possible others have a similar pile of glue for their distro, including
generic parts, and their generic parts are more polished than ours.
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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
* [PATCH 0/8] infiniband: Remove semaphores
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
Hi,
These are a set of patches which removes semaphores from infiniband.
These are part of a bigger effort to eliminate all semaphores from the
linux kernel.
NB: A few semaphores which are counting ones are replaced with an open-coded
implementation by introducing a new type in 'include/rdma/ib_sa.h'. Need to
see if this can be programmed in a generic way using wait queues.
Thanks,
Binoy
Binoy Jayan (8):
IB/core: iwpm_nlmsg_request: Replace semaphore with completion
IB/core: Replace semaphore sm_sem with completion
IB/hns: Replace semaphore poll_sem with mutex
IB/mthca: Replace semaphore poll_sem with mutex
IB/isert: Replace semaphore sem with completion
IB/hns: Replace counting semaphore event_sem with wait condition
IB/mthca: Replace counting semaphore event_sem with wait condition
IB/mlx5: Replace counting semaphore sem with wait condition
drivers/infiniband/core/iwpm_msg.c | 8 ++++----
drivers/infiniband/core/iwpm_util.c | 7 +++----
drivers/infiniband/core/iwpm_util.h | 3 ++-
drivers/infiniband/core/user_mad.c | 14 ++++++++------
drivers/infiniband/hw/hns/hns_roce_cmd.c | 28 ++++++++++++++++++----------
drivers/infiniband/hw/hns/hns_roce_device.h | 6 ++++--
drivers/infiniband/hw/mlx5/main.c | 3 ++-
drivers/infiniband/hw/mlx5/mlx5_ib.h | 3 ++-
drivers/infiniband/hw/mlx5/mr.c | 28 +++++++++++++++++++---------
drivers/infiniband/hw/mthca/mthca_cmd.c | 22 +++++++++++++---------
drivers/infiniband/hw/mthca/mthca_cmd.h | 1 +
drivers/infiniband/hw/mthca/mthca_dev.h | 5 +++--
drivers/infiniband/ulp/isert/ib_isert.c | 6 +++---
drivers/infiniband/ulp/isert/ib_isert.h | 3 ++-
include/rdma/ib_sa.h | 5 +++++
15 files changed, 89 insertions(+), 53 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 1/8] IB/core: iwpm_nlmsg_request: Replace semaphore with completion
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Semaphore sem in iwpm_nlmsg_request is used as completion, so
convert it to a struct completion type. Semaphores are going
away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/core/iwpm_msg.c | 8 ++++----
drivers/infiniband/core/iwpm_util.c | 7 +++----
drivers/infiniband/core/iwpm_util.h | 3 ++-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/core/iwpm_msg.c b/drivers/infiniband/core/iwpm_msg.c
index 1c41b95..761358f 100644
--- a/drivers/infiniband/core/iwpm_msg.c
+++ b/drivers/infiniband/core/iwpm_msg.c
@@ -394,7 +394,7 @@ int iwpm_register_pid_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found nlmsg_request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_register_pid_cb);
@@ -463,7 +463,7 @@ int iwpm_add_mapping_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_add_mapping_cb);
@@ -555,7 +555,7 @@ int iwpm_add_and_query_mapping_cb(struct sk_buff *skb,
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_add_and_query_mapping_cb);
@@ -749,7 +749,7 @@ int iwpm_mapping_error_cb(struct sk_buff *skb, struct netlink_callback *cb)
/* always for found request */
kref_put(&nlmsg_request->kref, iwpm_free_nlmsg_request);
barrier();
- up(&nlmsg_request->sem);
+ complete(&nlmsg_request->comp);
return 0;
}
EXPORT_SYMBOL(iwpm_mapping_error_cb);
diff --git a/drivers/infiniband/core/iwpm_util.c b/drivers/infiniband/core/iwpm_util.c
index ade71e7..08ddd2e 100644
--- a/drivers/infiniband/core/iwpm_util.c
+++ b/drivers/infiniband/core/iwpm_util.c
@@ -323,8 +323,7 @@ struct iwpm_nlmsg_request *iwpm_get_nlmsg_request(__u32 nlmsg_seq,
nlmsg_request->nl_client = nl_client;
nlmsg_request->request_done = 0;
nlmsg_request->err_code = 0;
- sema_init(&nlmsg_request->sem, 1);
- down(&nlmsg_request->sem);
+ init_completion(&nlmsg_request->comp);
return nlmsg_request;
}
@@ -368,8 +367,8 @@ int iwpm_wait_complete_req(struct iwpm_nlmsg_request *nlmsg_request)
{
int ret;
- ret = down_timeout(&nlmsg_request->sem, IWPM_NL_TIMEOUT);
- if (ret) {
+ ret = wait_for_completion_timeout(&nlmsg_request->comp, IWPM_NL_TIMEOUT);
+ if (!ret) {
ret = -EINVAL;
pr_info("%s: Timeout %d sec for netlink request (seq = %u)\n",
__func__, (IWPM_NL_TIMEOUT/HZ), nlmsg_request->nlmsg_seq);
diff --git a/drivers/infiniband/core/iwpm_util.h b/drivers/infiniband/core/iwpm_util.h
index af1fc14..ea6c299 100644
--- a/drivers/infiniband/core/iwpm_util.h
+++ b/drivers/infiniband/core/iwpm_util.h
@@ -43,6 +43,7 @@
#include <linux/delay.h>
#include <linux/workqueue.h>
#include <linux/mutex.h>
+#include <linux/completion.h>
#include <linux/jhash.h>
#include <linux/kref.h>
#include <net/netlink.h>
@@ -69,7 +70,7 @@ struct iwpm_nlmsg_request {
u8 nl_client;
u8 request_done;
u16 err_code;
- struct semaphore sem;
+ struct completion comp;
struct kref kref;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
* [PATCH 2/8] IB/core: Replace semaphore sm_sem with completion
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>
The semaphore 'sm_sem' is used as completion, so convert it to
struct completion. Semaphores are going away in the future. The initial
status of the completion variable is marked as completed by a call to
the function 'complete' immediately following the initialization.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/core/user_mad.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 415a318..df070cc 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -47,6 +47,7 @@
#include <linux/kref.h>
#include <linux/compat.h>
#include <linux/sched.h>
+#include <linux/completion.h>
#include <linux/semaphore.h>
#include <linux/slab.h>
@@ -87,7 +88,7 @@ struct ib_umad_port {
struct cdev sm_cdev;
struct device *sm_dev;
- struct semaphore sm_sem;
+ struct completion sm_comp;
struct mutex file_mutex;
struct list_head file_list;
@@ -1030,12 +1031,12 @@ static int ib_umad_sm_open(struct inode *inode, struct file *filp)
port = container_of(inode->i_cdev, struct ib_umad_port, sm_cdev);
if (filp->f_flags & O_NONBLOCK) {
- if (down_trylock(&port->sm_sem)) {
+ if (!try_wait_for_completion(&port->sm_comp)) {
ret = -EAGAIN;
goto fail;
}
} else {
- if (down_interruptible(&port->sm_sem)) {
+ if (wait_for_completion_interruptible(&port->sm_comp)) {
ret = -ERESTARTSYS;
goto fail;
}
@@ -1060,7 +1061,7 @@ static int ib_umad_sm_open(struct inode *inode, struct file *filp)
ib_modify_port(port->ib_dev, port->port_num, 0, &props);
err_up_sem:
- up(&port->sm_sem);
+ complete(&port->sm_comp);
fail:
return ret;
@@ -1079,7 +1080,7 @@ static int ib_umad_sm_close(struct inode *inode, struct file *filp)
ret = ib_modify_port(port->ib_dev, port->port_num, 0, &props);
mutex_unlock(&port->file_mutex);
- up(&port->sm_sem);
+ complete(&port->sm_comp);
kobject_put(&port->umad_dev->kobj);
@@ -1177,7 +1178,8 @@ static int ib_umad_init_port(struct ib_device *device, int port_num,
port->ib_dev = device;
port->port_num = port_num;
- sema_init(&port->sm_sem, 1);
+ init_completion(&port->sm_comp);
+ complete(&port->sm_comp);
mutex_init(&port->file_mutex);
INIT_LIST_HEAD(&port->file_list);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 3/8] IB/hns: Replace semaphore poll_sem with mutex
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The semaphore 'poll_sem' is a simple mutex, so it should be written as one.
Semaphores are going away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 12 ++++++------
drivers/infiniband/hw/hns/hns_roce_device.h | 3 ++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 2a0b6c0..1421fdb 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -119,7 +119,7 @@ static int hns_roce_cmd_mbox_post_hw(struct hns_roce_dev *hr_dev, u64 in_param,
return ret;
}
-/* this should be called with "poll_sem" */
+/* this should be called with "poll_mutex" */
static int __hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
u64 out_param, unsigned long in_modifier,
u8 op_modifier, u16 op,
@@ -167,10 +167,10 @@ static int hns_roce_cmd_mbox_poll(struct hns_roce_dev *hr_dev, u64 in_param,
{
int ret;
- down(&hr_dev->cmd.poll_sem);
+ mutex_lock(&hr_dev->cmd.poll_mutex);
ret = __hns_roce_cmd_mbox_poll(hr_dev, in_param, out_param, in_modifier,
op_modifier, op, timeout);
- up(&hr_dev->cmd.poll_sem);
+ mutex_unlock(&hr_dev->cmd.poll_mutex);
return ret;
}
@@ -275,7 +275,7 @@ int hns_roce_cmd_init(struct hns_roce_dev *hr_dev)
struct device *dev = &hr_dev->pdev->dev;
mutex_init(&hr_dev->cmd.hcr_mutex);
- sema_init(&hr_dev->cmd.poll_sem, 1);
+ mutex_init(&hr_dev->cmd.poll_mutex);
hr_dev->cmd.use_events = 0;
hr_dev->cmd.toggle = 1;
hr_dev->cmd.max_cmds = CMD_MAX_NUM;
@@ -319,7 +319,7 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
hr_cmd->token_mask = CMD_TOKEN_MASK;
hr_cmd->use_events = 1;
- down(&hr_cmd->poll_sem);
+ mutex_lock(&hr_cmd->poll_mutex);
return 0;
}
@@ -335,7 +335,7 @@ void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
down(&hr_cmd->event_sem);
kfree(hr_cmd->context);
- up(&hr_cmd->poll_sem);
+ mutex_unlock(&hr_cmd->poll_mutex);
}
struct hns_roce_cmd_mailbox
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 3417315..2afe075 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -34,6 +34,7 @@
#define _HNS_ROCE_DEVICE_H
#include <rdma/ib_verbs.h>
+#include <linux/mutex.h>
#define DRV_NAME "hns_roce"
@@ -358,7 +359,7 @@ struct hns_roce_cmdq {
struct dma_pool *pool;
u8 __iomem *hcr;
struct mutex hcr_mutex;
- struct semaphore poll_sem;
+ struct mutex poll_mutex;
/*
* Event mode: cmd register mutex protection,
* ensure to not exceed max_cmds and user use limit region
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
* [PATCH 4/8] IB/mthca: Replace semaphore poll_sem with mutex
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>
The semaphore 'poll_sem' is a simple mutex, so it should be written as one.
Semaphores are going away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mthca/mthca_cmd.c | 10 +++++-----
drivers/infiniband/hw/mthca/mthca_cmd.h | 1 +
drivers/infiniband/hw/mthca/mthca_dev.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index c7f49bb..0cb58ea 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -347,7 +347,7 @@ static int mthca_cmd_poll(struct mthca_dev *dev,
unsigned long end;
u8 status;
- down(&dev->cmd.poll_sem);
+ mutex_lock(&dev->cmd.poll_mutex);
err = mthca_cmd_post(dev, in_param,
out_param ? *out_param : 0,
@@ -382,7 +382,7 @@ static int mthca_cmd_poll(struct mthca_dev *dev,
}
out:
- up(&dev->cmd.poll_sem);
+ mutex_unlock(&dev->cmd.poll_mutex);
return err;
}
@@ -520,7 +520,7 @@ static int mthca_cmd_imm(struct mthca_dev *dev,
int mthca_cmd_init(struct mthca_dev *dev)
{
mutex_init(&dev->cmd.hcr_mutex);
- sema_init(&dev->cmd.poll_sem, 1);
+ mutex_init(&dev->cmd.poll_mutex);
dev->cmd.flags = 0;
dev->hcr = ioremap(pci_resource_start(dev->pdev, 0) + MTHCA_HCR_BASE,
@@ -582,7 +582,7 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
dev->cmd.flags |= MTHCA_CMD_USE_EVENTS;
- down(&dev->cmd.poll_sem);
+ mutex_lock(&dev->cmd.poll_mutex);
return 0;
}
@@ -601,7 +601,7 @@ void mthca_cmd_use_polling(struct mthca_dev *dev)
kfree(dev->cmd.context);
- up(&dev->cmd.poll_sem);
+ mutex_unlock(&dev->cmd.poll_mutex);
}
struct mthca_mailbox *mthca_alloc_mailbox(struct mthca_dev *dev,
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.h b/drivers/infiniband/hw/mthca/mthca_cmd.h
index d2e5b19..a7f197e 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.h
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.h
@@ -35,6 +35,7 @@
#ifndef MTHCA_CMD_H
#define MTHCA_CMD_H
+#include <linux/mutex.h>
#include <rdma/ib_verbs.h>
#define MTHCA_MAILBOX_SIZE 4096
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 4393a02..87ab964 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -120,7 +120,7 @@ enum {
struct mthca_cmd {
struct pci_pool *pool;
struct mutex hcr_mutex;
- struct semaphore poll_sem;
+ struct mutex poll_mutex;
struct semaphore event_sem;
int max_cmds;
spinlock_t context_lock;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 5/8] IB/isert: Replace semaphore sem with completion
From: Binoy Jayan @ 2016-10-17 16:30 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The semaphore 'sem' in isert_device is used as completion, so convert
it to struct completion. Semaphores are going away in the future.
Signed-off-by: Binoy Jayan <binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/infiniband/ulp/isert/ib_isert.c | 6 +++---
drivers/infiniband/ulp/isert/ib_isert.h | 3 ++-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/ulp/isert/ib_isert.c b/drivers/infiniband/ulp/isert/ib_isert.c
index 6dd43f6..de80f56 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.c
+++ b/drivers/infiniband/ulp/isert/ib_isert.c
@@ -619,7 +619,7 @@
mutex_unlock(&isert_np->mutex);
isert_info("np %p: Allow accept_np to continue\n", isert_np);
- up(&isert_np->sem);
+ complete(&isert_np->comp);
}
static void
@@ -2311,7 +2311,7 @@ struct rdma_cm_id *
isert_err("Unable to allocate struct isert_np\n");
return -ENOMEM;
}
- sema_init(&isert_np->sem, 0);
+ init_completion(&isert_np->comp);
mutex_init(&isert_np->mutex);
INIT_LIST_HEAD(&isert_np->accepted);
INIT_LIST_HEAD(&isert_np->pending);
@@ -2427,7 +2427,7 @@ struct rdma_cm_id *
int ret;
accept_wait:
- ret = down_interruptible(&isert_np->sem);
+ ret = wait_for_completion_interruptible(&isert_np->comp);
if (ret)
return -ENODEV;
diff --git a/drivers/infiniband/ulp/isert/ib_isert.h b/drivers/infiniband/ulp/isert/ib_isert.h
index c02ada5..a1277c0 100644
--- a/drivers/infiniband/ulp/isert/ib_isert.h
+++ b/drivers/infiniband/ulp/isert/ib_isert.h
@@ -3,6 +3,7 @@
#include <linux/in6.h>
#include <rdma/ib_verbs.h>
#include <rdma/rdma_cm.h>
+#include <linux/completion.h>
#include <rdma/rw.h>
#include <scsi/iser.h>
@@ -190,7 +191,7 @@ struct isert_device {
struct isert_np {
struct iscsi_np *np;
- struct semaphore sem;
+ struct completion comp;
struct rdma_cm_id *cm_id;
struct mutex mutex;
struct list_head accepted;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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
* [PATCH 6/8] IB/hns: Replace counting semaphore event_sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>
Counting semaphores are going away in the future, so replace the semaphore
hns_roce_cmdq::event_sem with an open-coded implementation.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/hns/hns_roce_cmd.c | 16 ++++++++++++----
drivers/infiniband/hw/hns/hns_roce_device.h | 3 ++-
include/rdma/ib_sa.h | 5 +++++
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 1421fdb..3e76717 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -248,10 +248,14 @@ static int hns_roce_cmd_mbox_wait(struct hns_roce_dev *hr_dev, u64 in_param,
{
int ret = 0;
- down(&hr_dev->cmd.event_sem);
+ wait_event(hr_dev->cmd.event_sem.wq,
+ atomic_add_unless(&hr_dev->cmd.event_sem.count, -1, 0));
+
ret = __hns_roce_cmd_mbox_wait(hr_dev, in_param, out_param,
in_modifier, op_modifier, op, timeout);
- up(&hr_dev->cmd.event_sem);
+
+ if (atomic_inc_return(&hr_dev->cmd.event_sem.count) == 1)
+ wake_up(&hr_dev->cmd.event_sem.wq);
return ret;
}
@@ -313,7 +317,9 @@ int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
hr_cmd->context[hr_cmd->max_cmds - 1].next = -1;
hr_cmd->free_head = 0;
- sema_init(&hr_cmd->event_sem, hr_cmd->max_cmds);
+ init_waitqueue_head(&hr_cmd->event_sem.wq);
+ atomic_set(&hr_cmd->event_sem.count, hr_cmd->max_cmds);
+
spin_lock_init(&hr_cmd->context_lock);
hr_cmd->token_mask = CMD_TOKEN_MASK;
@@ -332,7 +338,9 @@ void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)
hr_cmd->use_events = 0;
for (i = 0; i < hr_cmd->max_cmds; ++i)
- down(&hr_cmd->event_sem);
+ wait_event(hr_cmd->event_sem.wq,
+ atomic_add_unless(
+ &hr_dev->cmd.event_sem.count, -1, 0));
kfree(hr_cmd->context);
mutex_unlock(&hr_cmd->poll_mutex);
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 2afe075..6aed04a 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -34,6 +34,7 @@
#define _HNS_ROCE_DEVICE_H
#include <rdma/ib_verbs.h>
+#include <rdma/ib_sa.h>
#include <linux/mutex.h>
#define DRV_NAME "hns_roce"
@@ -364,7 +365,7 @@ struct hns_roce_cmdq {
* Event mode: cmd register mutex protection,
* ensure to not exceed max_cmds and user use limit region
*/
- struct semaphore event_sem;
+ struct ib_semaphore event_sem;
int max_cmds;
spinlock_t context_lock;
int free_head;
diff --git a/include/rdma/ib_sa.h b/include/rdma/ib_sa.h
index 5ee7aab..1901042 100644
--- a/include/rdma/ib_sa.h
+++ b/include/rdma/ib_sa.h
@@ -291,6 +291,11 @@ struct ib_sa_service_rec {
#define IB_SA_GUIDINFO_REC_GID6 IB_SA_COMP_MASK(10)
#define IB_SA_GUIDINFO_REC_GID7 IB_SA_COMP_MASK(11)
+struct ib_semaphore {
+ wait_queue_head_t wq;
+ atomic_t count;
+};
+
struct ib_sa_guidinfo_rec {
__be16 lid;
u8 block_num;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 7/8] IB/mthca: Replace counting semaphore event_sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>
Counting semaphores are going away in the future, so replace the semaphore
mthca_cmd::event_sem with an open-coded implementation.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mthca/mthca_cmd.c | 12 ++++++++----
drivers/infiniband/hw/mthca/mthca_dev.h | 3 ++-
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_cmd.c b/drivers/infiniband/hw/mthca/mthca_cmd.c
index 0cb58ea..5b8d5ea 100644
--- a/drivers/infiniband/hw/mthca/mthca_cmd.c
+++ b/drivers/infiniband/hw/mthca/mthca_cmd.c
@@ -417,7 +417,8 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
int err = 0;
struct mthca_cmd_context *context;
- down(&dev->cmd.event_sem);
+ wait_event(dev->cmd.event_sem.wq,
+ atomic_add_unless(&dev->cmd.event_sem.count, -1, 0));
spin_lock(&dev->cmd.context_lock);
BUG_ON(dev->cmd.free_head < 0);
@@ -459,7 +460,8 @@ static int mthca_cmd_wait(struct mthca_dev *dev,
dev->cmd.free_head = context - dev->cmd.context;
spin_unlock(&dev->cmd.context_lock);
- up(&dev->cmd.event_sem);
+ if (atomic_inc_return(&dev->cmd.event_sem.count) == 1)
+ wake_up(&dev->cmd.event_sem.wq);
return err;
}
@@ -571,7 +573,8 @@ int mthca_cmd_use_events(struct mthca_dev *dev)
dev->cmd.context[dev->cmd.max_cmds - 1].next = -1;
dev->cmd.free_head = 0;
- sema_init(&dev->cmd.event_sem, dev->cmd.max_cmds);
+ init_waitqueue_head(&dev->cmd.event_sem.wq);
+ atomic_set(&dev->cmd.event_sem.count, dev->cmd.max_cmds);
spin_lock_init(&dev->cmd.context_lock);
for (dev->cmd.token_mask = 1;
@@ -597,7 +600,8 @@ void mthca_cmd_use_polling(struct mthca_dev *dev)
dev->cmd.flags &= ~MTHCA_CMD_USE_EVENTS;
for (i = 0; i < dev->cmd.max_cmds; ++i)
- down(&dev->cmd.event_sem);
+ wait_event(dev->cmd.event_sem.wq,
+ atomic_add_unless(&dev->cmd.event_sem.count, -1, 0));
kfree(dev->cmd.context);
diff --git a/drivers/infiniband/hw/mthca/mthca_dev.h b/drivers/infiniband/hw/mthca/mthca_dev.h
index 87ab964..1f88835c 100644
--- a/drivers/infiniband/hw/mthca/mthca_dev.h
+++ b/drivers/infiniband/hw/mthca/mthca_dev.h
@@ -46,6 +46,7 @@
#include <linux/list.h>
#include <linux/semaphore.h>
+#include <rdma/ib_sa.h>
#include "mthca_provider.h"
#include "mthca_doorbell.h"
@@ -121,7 +122,7 @@ struct mthca_cmd {
struct pci_pool *pool;
struct mutex hcr_mutex;
struct mutex poll_mutex;
- struct semaphore event_sem;
+ struct ib_semaphore event_sem;
int max_cmds;
spinlock_t context_lock;
int free_head;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH 8/8] IB/mlx5: Replace counting semaphore sem with wait condition
From: Binoy Jayan @ 2016-10-17 16:31 UTC (permalink / raw)
To: Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma, linux-kernel, Binoy Jayan
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan@linaro.org>
Counting semaphores are going away in the future, so replace the semaphore
umr_common::sem with an open-coded implementation.
Signed-off-by: Binoy Jayan <binoy.jayan@linaro.org>
---
drivers/infiniband/hw/mlx5/main.c | 3 ++-
drivers/infiniband/hw/mlx5/mlx5_ib.h | 3 ++-
drivers/infiniband/hw/mlx5/mr.c | 28 +++++++++++++++++++---------
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 2217477..5667ea8 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -2520,7 +2520,8 @@ static int create_umr_res(struct mlx5_ib_dev *dev)
dev->umrc.cq = cq;
dev->umrc.pd = pd;
- sema_init(&dev->umrc.sem, MAX_UMR_WR);
+ init_waitqueue_head(&dev->umrc.sem.wq);
+ atomic_set(&dev->umrc.sem.count, MAX_UMR_WR);
ret = mlx5_mr_cache_init(dev);
if (ret) {
mlx5_ib_warn(dev, "mr cache init failed %d\n", ret);
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index dcdcd19..60e2d29 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -45,6 +45,7 @@
#include <linux/mlx5/transobj.h>
#include <rdma/ib_user_verbs.h>
#include <rdma/mlx5-abi.h>
+#include <rdma/ib_sa.h>
#define mlx5_ib_dbg(dev, format, arg...) \
pr_debug("%s:%s:%d:(pid %d): " format, (dev)->ib_dev.name, __func__, \
@@ -533,7 +534,7 @@ struct umr_common {
struct ib_qp *qp;
/* control access to UMR QP
*/
- struct semaphore sem;
+ struct ib_semaphore sem;
};
enum {
diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index d4ad672..7c2af26 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -900,7 +900,9 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
prep_umr_reg_wqe(pd, &umrwr.wr, &sg, dma, npages, mr->mmkey.key,
page_shift, virt_addr, len, access_flags);
- down(&umrc->sem);
+ wait_event(umrc->sem.wq,
+ atomic_add_unless(&umrc->sem.count, -1, 0));
+
err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
if (err) {
mlx5_ib_warn(dev, "post send failed, err %d\n", err);
@@ -920,7 +922,8 @@ static struct mlx5_ib_mr *reg_umr(struct ib_pd *pd, struct ib_umem *umem,
mr->live = 1;
unmap_dma:
- up(&umrc->sem);
+ if (atomic_inc_return(&umrc->sem.count) == 1)
+ wake_up(&umrc->sem.wq);
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
kfree(mr_pas);
@@ -1031,7 +1034,8 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
wr.mkey = mr->mmkey.key;
wr.target.offset = start_page_index;
- down(&umrc->sem);
+ wait_event(umrc->sem.wq,
+ atomic_add_unless(&umrc->sem.count, -1, 0));
err = ib_post_send(umrc->qp, &wr.wr, &bad);
if (err) {
mlx5_ib_err(dev, "UMR post send failed, err %d\n", err);
@@ -1043,7 +1047,8 @@ int mlx5_ib_update_mtt(struct mlx5_ib_mr *mr, u64 start_page_index, int npages,
err = -EFAULT;
}
}
- up(&umrc->sem);
+ if (atomic_inc_return(&umrc->sem.count) == 1)
+ wake_up(&umrc->sem.wq);
}
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
@@ -1224,15 +1229,18 @@ static int unreg_umr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)
umrwr.wr.wr_cqe = &umr_context.cqe;
prep_umr_unreg_wqe(dev, &umrwr.wr, mr->mmkey.key);
- down(&umrc->sem);
+ wait_event(umrc->sem.wq,
+ atomic_add_unless(&umrc->sem.count, -1, 0));
err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
if (err) {
- up(&umrc->sem);
+ if (atomic_inc_return(&umrc->sem.count) == 1)
+ wake_up(&umrc->sem.wq);
mlx5_ib_dbg(dev, "err %d\n", err);
goto error;
} else {
wait_for_completion(&umr_context.done);
- up(&umrc->sem);
+ if (atomic_inc_return(&umrc->sem.count) == 1)
+ wake_up(&umrc->sem.wq);
}
if (umr_context.status != IB_WC_SUCCESS) {
mlx5_ib_warn(dev, "unreg umr failed\n");
@@ -1291,7 +1299,8 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
}
/* post send request to UMR QP */
- down(&umrc->sem);
+ wait_event(umrc->sem.wq,
+ atomic_add_unless(&umrc->sem.count, -1, 0));
err = ib_post_send(umrc->qp, &umrwr.wr, &bad);
if (err) {
@@ -1305,7 +1314,8 @@ static int rereg_umr(struct ib_pd *pd, struct mlx5_ib_mr *mr, u64 virt_addr,
}
}
- up(&umrc->sem);
+ if (atomic_inc_return(&umrc->sem.count) == 1)
+ wake_up(&umrc->sem.wq);
if (flags & IB_MR_REREG_TRANS) {
dma_unmap_single(ddev, dma, size, DMA_TO_DEVICE);
kfree(mr_pas);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: iscsi_trx going into D state
From: Robert LeBlanc @ 2016-10-17 16:32 UTC (permalink / raw)
To: Zhu Lingshan; +Cc: linux-rdma, linux-scsi
In-Reply-To: <880e374c-cf8b-f276-e930-57b09fe1a686@suse.com>
Some more info as we hit this this morning. We have volumes mirrored
between two targets and we had one target on the kernel with the three
patches mentioned in this thread [0][1][2] and the other was on a
kernel without the patches. We decided that after a week and a half we
wanted to get both targets on the same kernel so we rebooted the
non-patched target. Within an hour we saw iSCSI in D state with the
same stack trace so it seems that we are not hitting any of the
WARN_ON lines. We are getting both iscsi_trx and iscsi_np both in D
state, this time we have two iscsi_trx processes in D state. I don't
know if stale sessions on the clients could be contributing to this
issue (the target trying to close non-existent sessions??). This is on
4.4.23. Any more debug info we can throw at this problem to help?
Thank you,
Robert LeBlanc
# ps aux | grep D | grep iscsi
root 16525 0.0 0.0 0 0 ? D 08:50 0:00 [iscsi_np]
root 16614 0.0 0.0 0 0 ? D 08:50 0:00 [iscsi_trx]
root 16674 0.0 0.0 0 0 ? D 08:50 0:00 [iscsi_trx]
# for i in 16525 16614 16674; do echo $i; cat /proc/$i/stack; done
16525
[<ffffffff814f0d5f>] iscsit_stop_session+0x19f/0x1d0
[<ffffffff814e2516>] iscsi_check_for_session_reinstatement+0x1e6/0x270
[<ffffffff814e4ed0>] iscsi_target_check_for_existing_instances+0x30/0x40
[<ffffffff814e5020>] iscsi_target_do_login+0x140/0x640
[<ffffffff814e63bc>] iscsi_target_start_negotiation+0x1c/0xb0
[<ffffffff814e410b>] iscsi_target_login_thread+0xa9b/0xfc0
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff
16614
[<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
[<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
[<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
[<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
[<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff
16674
[<ffffffff814cca79>] target_wait_for_sess_cmds+0x49/0x1a0
[<ffffffffa064692b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
[<ffffffff814f0ef2>] iscsit_close_connection+0x162/0x870
[<ffffffff814df9bf>] iscsit_take_action_for_connection_exit+0x7f/0x100
[<ffffffff814f00a0>] iscsi_target_rx_thread+0x5a0/0xe80
[<ffffffff8109c748>] kthread+0xd8/0xf0
[<ffffffff8172018f>] ret_from_fork+0x3f/0x70
[<ffffffffffffffff>] 0xffffffffffffffff
[0] https://www.spinics.net/lists/target-devel/msg13463.html
[1] http://marc.info/?l=linux-scsi&m=147282568910535&w=2
[2] http://www.spinics.net/lists/linux-scsi/msg100221.html
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
On Fri, Oct 7, 2016 at 8:59 PM, Zhu Lingshan <lszhu@suse.com> wrote:
> Hi Robert,
>
> I also see this issue, but this is not the only code path can trigger this
> problem, I think you may also see iscsi_np in D status. I fixed one code
> path whitch still not merged to mainline. I will forward you my patch later.
> Note: my patch only fixed one code path, you may see other call statck with
> D status.
>
> Thanks,
> BR
> Zhu Lingshan
>
>
> 在 2016/10/1 1:14, Robert LeBlanc 写道:
>>
>> We are having a reoccurring problem where iscsi_trx is going into D
>> state. It seems like it is waiting for a session tear down to happen
>> or something, but keeps waiting. We have to reboot these targets on
>> occasion. This is running the 4.4.12 kernel and we have seen it on
>> several previous 4.4.x and 4.2.x kernels. There is no message in dmesg
>> or /var/log/messages. This seems to happen with increased frequency
>> when we have a disruption in our Infiniband fabric, but can happen
>> without any changes to the fabric (other than hosts rebooting).
>>
>> # ps aux | grep iscsi | grep D
>> root 4185 0.0 0.0 0 0 ? D Sep29 0:00
>> [iscsi_trx]
>> root 18505 0.0 0.0 0 0 ? D Sep29 0:00
>> [iscsi_np]
>>
>> # cat /proc/4185/stack
>> [<ffffffff814cc999>] target_wait_for_sess_cmds+0x49/0x1a0
>> [<ffffffffa087292b>] isert_wait_conn+0x1ab/0x2f0 [ib_isert]
>> [<ffffffff814f0de2>] iscsit_close_connection+0x162/0x840
>> [<ffffffff814df8df>] iscsit_take_action_for_connection_exit+0x7f/0x100
>> [<ffffffff814effc0>] iscsi_target_rx_thread+0x5a0/0xe80
>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> [<ffffffffffffffff>] 0xffffffffffffffff
>>
>> # cat /proc/18505/stack
>> [<ffffffff814f0c71>] iscsit_stop_session+0x1b1/0x1c0
>> [<ffffffff814e2436>] iscsi_check_for_session_reinstatement+0x1e6/0x270
>> [<ffffffff814e4df0>] iscsi_target_check_for_existing_instances+0x30/0x40
>> [<ffffffff814e4f40>] iscsi_target_do_login+0x140/0x640
>> [<ffffffff814e62dc>] iscsi_target_start_negotiation+0x1c/0xb0
>> [<ffffffff814e402b>] iscsi_target_login_thread+0xa9b/0xfc0
>> [<ffffffff8109c6f8>] kthread+0xd8/0xf0
>> [<ffffffff8172004f>] ret_from_fork+0x3f/0x70
>> [<ffffffffffffffff>] 0xffffffffffffffff
>>
>> What can we do to help get this resolved?
>>
>> Thanks,
>>
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" 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: [PATCH 6/8] IB/hns: Replace counting semaphore event_sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:39 UTC (permalink / raw)
To: Binoy Jayan
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-7-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Mon, Oct 17, 2016 at 10:01:00PM +0530, Binoy Jayan wrote:
> Counting semaphores are going away in the future, so replace the semaphore
> hns_roce_cmdq::event_sem with an open-coded implementation.
Sorry, but no. Using a proper semaphore abstraction is always
better than open coding it. While most semaphore users should move
to better primitives, those that actually are counting semaphore
should stick to the existing primitive.
--
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 7/8] IB/mthca: Replace counting semaphore event_sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:39 UTC (permalink / raw)
To: Binoy Jayan
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
linux-rdma, linux-kernel
In-Reply-To: <1476721862-7070-8-git-send-email-binoy.jayan@linaro.org>
Same NAK as for the last patch.
^ permalink raw reply
* Re: [PATCH 8/8] IB/mlx5: Replace counting semaphore sem with wait condition
From: Christoph Hellwig @ 2016-10-17 16:40 UTC (permalink / raw)
To: Binoy Jayan
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Arnd Bergmann,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-9-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Mon, Oct 17, 2016 at 10:01:02PM +0530, Binoy Jayan wrote:
> Counting semaphores are going away in the future, so replace the semaphore
> umr_common::sem with an open-coded implementation.
And NAK again..
--
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/8] infiniband: Remove semaphores
From: Bart Van Assche @ 2016-10-17 16:57 UTC (permalink / raw)
To: Binoy Jayan, Doug Ledford, Sean Hefty, Hal Rosenstock
Cc: Arnd Bergmann, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1476721862-7070-1-git-send-email-binoy.jayan-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On 10/17/2016 09:30 AM, Binoy Jayan wrote:
> These are a set of patches which removes semaphores from infiniband.
> These are part of a bigger effort to eliminate all semaphores from the
> linux kernel.
Hello Binoy,
Why do you think it would be a good idea to eliminate all semaphores
from the Linux kernel? I don't know anyone who doesn't consider
semaphores a useful abstraction.
Thanks,
Bart.
--
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-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jason Gunthorpe @ 2016-10-17 17:31 UTC (permalink / raw)
To: Doug Ledford; +Cc: Jarod Wilson, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1e03ceea-5584-6bbb-6fbe-e11d9d251649-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Sun, Oct 16, 2016 at 10:40:27AM -0400, Doug Ledford wrote:
> >> +Requires=rdma.service
> >> +After=rdma.service opensm.service
> >
> > This is the only RH specific thiing I see.. Could we standardize on
> > something here and use it on all distros? rdma-available.target?
>
> You can't, unless you rename the rdma.service unit file to something
> else. They are tied in that way.
Well, I don't really care about names too much, rdma-whatever.target
is fine...
> >> +++ b/glue/redhat/rdma.cxgb4.sys.modprobe
> >> @@ -0,0 +1 @@
> >> +install cxgb4 /sbin/modprobe --ignore-install cxgb4 $CMDLINE_OPTS && /sbin/modprobe iw_cxgb4
> >
> > What are these for? Should they be cross distro? Why are only a few
> > drivers this special?
>
> We have one of these for every two (or more) part driver. They aren't
> special, it's just the multipart drivers that are.
So, should we move them into the provider directories? Or patch some
kind of request_module into the kernel?
> > I wonder if this could be split into a generic 'load the modules' part
> > and a distro specific part? Every distro needs systemd to load the
> > extra modules because out auto-loading is broken - IMHO, and that is
> > pretty complex unfortunately.
>
> Yes, this probably could be broken out.
So, I think the 'systemd way' would be a rdma-load-modules.service
onshot and a rdma-whatever.target
This way a distro can add their other stuff with additional drops ins,
eg rdma-bios-fixup.service (after load-modules, before
rdma-whatever.target)
> >> +[Unit]
> >> +Description=Initialize the iWARP/InfiniBand/RDMA stack in the kernel
> >> +Documentation=file:/etc/rdma/rdma.conf
> >> +RefuseManualStop=true
> >> +DefaultDependencies=false
> >> +Conflicts=emergency.target emergency.service
> >> +Before=network.target remote-fs-pre.target
> >
> > This is an area we really need to cross-distro standardize - we really
> > need a set of rdma-*.targets.
> >
> > eg
> > rdma-available.target
> > - RDMA hardware is available and all prep is done
> > opensm (if installed) is started, etc
> > Use in place of rdma.service
> > rdma-detected.target
> > - udev detected rdma hardware
>
> It's not that easy, unfortunately. Creating a target is a big deal.
Okay, do you mean big deal in the sense we need to get approval from
systemd folks or something? We are a big grown up subsystem now, and
good systemd integration is very important to a good user experience
these days.
I think we are in a better place now, because the target(s) *really*
needs to be cross distro and maintained 'upstream' - rdma-core is the
natural place to do that.
> I could be wrong). I would have thought it means "Start this unit
> before starting the target listed in the Before= line", instead it
> means "Start this unit and make sure it finishes before the target
> in the Before= line is considered complete". It can be started
> after the listed target is started, but the listed target won't be
> considered complete until it is also complete.
I'm not sure I follow the issue?
Your description matches how I understand systemd - a .target will not
become ready until all the prerequisits reach a 'ready' state (eg a
oneshot script completes). As the target does not become 'ready' until
its prerequisites are all 'ready', and dependents never start until
the parent is 'ready', this provides a reliable ordering sequence
point in the startup.
The order of starting is simply that target prerequisites are started
before the target becomes ready.
When systemd enabling anything it is important to keep in mind the
distinction between 'started' and 'ready' - and broadly speaking, our
daemons do not do this correctly today :/.
So the design goal is to make a target(s) that indicates enough of the
RDMA core systems is 'ready' so that we can begin to start things that
use rdmacm, etc.
We have problems with our daemons not properly interacting with
systemd to indicate 'ready', and that will cause bugs, but the overall
idea should be sound.
So this is a sketch of what I am thinking about.
rdma-fix-bios.service:
[Unit]
Type=oneshot
Before=rdma-available.target, rdma-load-modules.service
rdma-load-modules.service:
[Unit]
Type=oneshot
Before=rdma-available.target
iwpmd.service:
[Unit]
After=rdma-load-modules.service
Before=rdma-available.target
opensm.service:
[Unit]
After=rdma-load-modules.service
Before=rdma-available.target
rdma-available.target:
[Unit]
Description=Target indicating that the RDMA kernel stack is setup for user use.
srp_daemon.service:
[Unit]
After=rdma-available.target
Before=remote-fs-pre.target
'Type=oneshot' will prevent anything past rdma-available.target from
starting until the scripts complete.
Internal ordering in the 'before' section has stuff like opensm and
iwpmd taken care of, and all 'user' daemons have a clear single
.target to depend on that works no matter what the distro or
underlying RDMA protocol.
To be clear, I'm proposing something like this as a goal, there will
certainly be some needed work on the C daemons to get there:
- iwpmd forks in the wrong place, it needs to fork after it sets up
netlink, or stop forking and use sd_notify. (or even better, we
should figure out how to use ListenNetlink !!)
- ibacmd needs to use socket activation/sd_notify/fork order to ensure
acm is started before rdma cm users start
- srp_daemon needs to respond to dynamic prefix changes and probably
use sd_notify/fork order to indicate that it is OK to move on to
mounting FS.
Why is this more important now?
1) There are more SM's than opensm, it makes those peoples lives
very hard if 'opensm' is hardcoded into all the service files for
correctness, hard to swap out opensm with something else. Eg hfi
does not use opensm.
2) iwarp is involved in all of this too, and we need to start iwpmd
before moving on to other services that might need rdmacm. Ditto
for ibacm
3) Things like rxe could use additional 'before' service plugins to
enable rxe mode on interfaces.
So, I think this is a subject worth tackling.. (over the long term,
let us not block Jarod's stuff)
The goal would be to standardize the .target names and be able to use
upstream .service files for many of the things, and allow
distros/users/other to reliably 'drop in' additional stuff (eg the
bios-fixup) at various well defined sequence points.
> Fortunately, the targets listed in the unit files are pretty standard
> (they are part of the systemd upstream), and so I think they can be
> cross distro just as they are.
Sure, the pre-existing targets are, it is stuff like opensm.service
that seems off to me.
> >> +Description=Start or stop the daemon that attaches to SRP devices
> >> +Documentation=file:///etc/rdma/rdma.conf file:///etc/srp_daemon.conf
> >> +DefaultDependencies=false
> >> +Conflicts=emergency.target emergency.service
> >> +Requires=rdma.service
> >> +Wants=opensm.service
> >> +After=rdma.service opensm.service
> >> +After=network.target
> >> +Before=remote-fs-pre.target
> >
> > Also should be common, why does it reference opensm.service?
>
> Because if opensm is running on this host, then it must be up before the
> configured srp targets are valid any time there is a non-default subnet
> prefix.
Well, that kinda sounds like a srp_daemon bug - how does it work
race-free with an external SM?
Even with an on-node opensm, how does this work without a race? Is
After=opensm.service enough to assert that opensm has completed a
sweep and assigned the subnet prefix?
If we can have srp_daemon respond to dynamic changes in the subnet
prefix can we drop this from the unit file?
> >> +[Service]
> >> +Type=simple
> >> +ExecStart=/usr/sbin/srp_daemon.sh
> >
> > Hurm, someday we have to make better systemd integration for these
> > daemons..
>
> There really isn't any better integration to get with our complex
> daemons unless we update the daemons themselves to get rid of their
> shell script starters...
Exactly, update the daemons.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jason Gunthorpe @ 2016-10-17 17:46 UTC (permalink / raw)
To: Jarod Wilson; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161017162221.GI14983-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Mon, Oct 17, 2016 at 12:22:21PM -0400, Jarod Wilson wrote:
> Worksforme. As mentioned in reply to Leon, I really don't care what the
> directory is called, just that we get this stuff out there, instead of
> being in our own custom glue package. I do think maybe Leon's suggestion
> of having a directory to put a bunch of distro-specific directories under
> would be good for cleanliness, but I also seem to recall that debian
> actually looks for a directory in the root of the tarball, so it may need
> to stay like it is.
Okay, let us just do a very minimal upstreaming and tackle things from
there..
> > Common stuff should be installed via cmake
> >
> > > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > > new file mode 100644
> > > index 0000000..1cd031a
> > > +++ b/glue/redhat/ibacm.service
> >
> > Can we just put this in ibacm/ ?
>
> Probably.
Okay, the only thing I really don't like being upstream is the
opensm.service..
Do you know why acm needs that?
> > > +++ b/glue/redhat/iwpmd.service
> > > @@ -0,0 +1,12 @@
> > > +[Unit]
> > > +Description=Starts the IWPMD daemon
> > > +Documentation=file:///usr/share/doc/iwpmd/README
> >
> > File does not exit? There is a man page now
> >
> > We already have a iwpmd/iwpmd.service that is almost identical, can you
> > you just update it and drop this version?
>
> Bah, that's a carryover from our individually packaged iwpmd, didn't look
> closely enough. Can we merge a bit from ours into the 'stock' one? The
> main relevant difference I see is we have ours set to load after
> syslog.target as well as network.target.
syslog.target is obsolete now, just drop it from all your unit files
unless you need to support systemd <= 35. systemd no longer even
documents this special target exists.
commit e8f2b5c11e9db0bab2654e75c2558955effb82fe
Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Mon Sep 12 15:53:10 2016 -0600
iwpmd: Remove syslog.target from service file
Debian's Lintian remarks:
W: rdma-plumbing: systemd-service-file-refers-to-obsolete-target lib/systemd/system/iwpmd.service syslog.target
Apparently systemd stopped recommending this in version 35, socket activation
eliminates the need.
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> > > +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> >
> > Also cross distro
>
> Yeah, these are definitely prime candidates for being cross-distro. And
> really, I was thinking maybe these should be part of the core upstream
> udev/systemd rules set, rather than something we ship here.
Okay. The trick will be to standardize the systemd_wants name ..
> > > +# When we detect a new verbs device is added to the system, set the node
> > > +# description on that device
> > > +# If rdma-ndd is installed, defer the setting of the node description to it.
> > > +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
> >
> > Shouldn't this udev drop-in by in the rdma-ndd package?
>
> Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
> Doug said about this one...
Guess I didn't read closely enough, this is used if rdma-ndd is not
installed..
So something like that should be upstream, but the 'sleep 1' is ugly -
this should probably be a systemd service that runs after
network-online.target not as a script run from udev?
rdma-ndd dynamically sets the NodeDescription to the hostname in the
adaptor for the subnet manager/tools to ready. I guess this hunk is
setting the NodeDescription one-shot at boot..
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH rdma-core 2/4] glue/redhat: add udev/systemd/etc infrastructure bits
From: Jarod Wilson @ 2016-10-17 18:20 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, Doug Ledford
In-Reply-To: <20161017174611.GB6430-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Mon, Oct 17, 2016 at 11:46:11AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 17, 2016 at 12:22:21PM -0400, Jarod Wilson wrote:
> > Worksforme. As mentioned in reply to Leon, I really don't care what the
> > directory is called, just that we get this stuff out there, instead of
> > being in our own custom glue package. I do think maybe Leon's suggestion
> > of having a directory to put a bunch of distro-specific directories under
> > would be good for cleanliness, but I also seem to recall that debian
> > actually looks for a directory in the root of the tarball, so it may need
> > to stay like it is.
>
> Okay, let us just do a very minimal upstreaming and tackle things from
> there..
Sounds like a plan.
> > > Common stuff should be installed via cmake
> > >
> > > > diff --git a/glue/redhat/ibacm.service b/glue/redhat/ibacm.service
> > > > new file mode 100644
> > > > index 0000000..1cd031a
> > > > +++ b/glue/redhat/ibacm.service
> > >
> > > Can we just put this in ibacm/ ?
> >
> > Probably.
>
> Okay, the only thing I really don't like being upstream is the
> opensm.service..
>
> Do you know why acm needs that?
I think Doug already attempted to address this elsewhere in the thread,
and he'd know better than me.
> > > > +++ b/glue/redhat/iwpmd.service
> > > > @@ -0,0 +1,12 @@
> > > > +[Unit]
> > > > +Description=Starts the IWPMD daemon
> > > > +Documentation=file:///usr/share/doc/iwpmd/README
> > >
> > > File does not exit? There is a man page now
> > >
> > > We already have a iwpmd/iwpmd.service that is almost identical, can you
> > > you just update it and drop this version?
> >
> > Bah, that's a carryover from our individually packaged iwpmd, didn't look
> > closely enough. Can we merge a bit from ours into the 'stock' one? The
> > main relevant difference I see is we have ours set to load after
> > syslog.target as well as network.target.
>
> syslog.target is obsolete now, just drop it from all your unit files
> unless you need to support systemd <= 35. systemd no longer even
> documents this special target exists.
>
> commit e8f2b5c11e9db0bab2654e75c2558955effb82fe
> Author: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Mon Sep 12 15:53:10 2016 -0600
>
> iwpmd: Remove syslog.target from service file
>
> Debian's Lintian remarks:
>
> W: rdma-plumbing: systemd-service-file-refers-to-obsolete-target lib/systemd/system/iwpmd.service syslog.target
>
> Apparently systemd stopped recommending this in version 35, socket activation
> eliminates the need.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Ah, okay. So not needed for any distro that is even remotely modern.
> > > > +SUBSYSTEM=="module", KERNEL=="cxgb*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="ib_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="mlx*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="iw_*", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="be2net", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > > > +SUBSYSTEM=="module", KERNEL=="enic", ACTION=="add", TAG+="systemd", ENV{SYSTEMD_WANTS}="rdma.service"
> > >
> > > Also cross distro
> >
> > Yeah, these are definitely prime candidates for being cross-distro. And
> > really, I was thinking maybe these should be part of the core upstream
> > udev/systemd rules set, rather than something we ship here.
>
> Okay. The trick will be to standardize the systemd_wants name ..
Perhaps rdma-core should go with rdma-core.service? We were shipping a
package called 'rdma' that carried that.
> > > > +# When we detect a new verbs device is added to the system, set the node
> > > > +# description on that device
> > > > +# If rdma-ndd is installed, defer the setting of the node description to it.
> > > > +SUBSYSTEM=="infiniband", KERNEL=="*", ACTION=="add", TEST!="/usr/sbin/rdma-ndd", RUN+="/bin/bash -c 'sleep 1; echo -n `hostname -s` %k > /sys/class/infiniband/%k/node_desc'"
> > >
> > > Shouldn't this udev drop-in by in the rdma-ndd package?
> >
> > Honestly don't even have a clue what rdma-ndd is. :) Don't remember what
> > Doug said about this one...
>
> Guess I didn't read closely enough, this is used if rdma-ndd is not
> installed..
>
> So something like that should be upstream, but the 'sleep 1' is ugly -
> this should probably be a systemd service that runs after
> network-online.target not as a script run from udev?
>
> rdma-ndd dynamically sets the NodeDescription to the hostname in the
> adaptor for the subnet manager/tools to ready. I guess this hunk is
> setting the NodeDescription one-shot at boot..
Rather than having this janky udev rule, what if we simply made rdma-ndd
part of what's installed with rdma-core, rather than something found in
yet another infiniband package? (Looks like it's in infiniband-diags,
wasn't even aware rdma-ndd existed until looking at this here).
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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: RQ overflow seen running isert traffic
From: Steve Wise @ 2016-10-17 18:29 UTC (permalink / raw)
To: 'Potnuri Bharat Teja', 'Sagi Grimberg'
Cc: target-devel, nab, linux-rdma
In-Reply-To: <20161017111655.GA21245@chelsio.com>
> On Wednesday, October 10/05/16, 2016 at 11:44:12 +0530, Sagi Grimberg wrote:
> >
> > > Hi Sagi,
> >
> > Hey Baharat,
> >
> > Sorry for the late response, its the holiday
> > season in Israel...
> >
> > > I've been trying to understand the isert functionality with respect to
> > > RDMA Receive Queue sizing and Queue full handling. Here is the problem
> > > is see with iw_cxgb4:
> > >
> > > After running few minutes of iSER traffic with iw_cxgb4, I am seeing
> > > post receive failures due to receive queue full returning -ENOMEM.
> > > In case of iw_cxgb4 the RQ size is 130 with qp attribute max_recv_wr =
129,
> > > passed down by isert to iw_cxgb4.isert decides on max_recv_wr as 129 based
> > > on (ISERT_QP_MAX_RECV_DTOS = ISCSI_DEF_XMIT_CMDS_MAX = 128) + 1.
> >
> > That's correct.
>
> Hi Sagi,
> Sorry for the late reply, I had to recheck my findings before I reply
> you back.
>
> My interpretation of the queue full issue was not complete, got carried
> away by the recieve queue and missed the SQ failure due to full among
> the debug logs.
>
> Here is what was happening:
> It was the SQ full first and fails to get posted with ENOMEM, due to this
> the command is queued to queue full list this will schedule it to post at
> later instance and this repeated try for posting will cause the rq to get
> full. For everyfurther try sq post will any way fail and an extra rq wr is
> posted as a part of datain() leading it to be full too.
>
> This happened a bit earlier in my case, since I corrected the the
> iser_put_datain() to return error to LIO.
>
> Here is the failure log on a fresh 4.8 kernel:
> isert: isert_rdma_rw_ctx_post: Cmd: ffff882ec8c96e60 failed to post RDMA
> res <===here is the post send failure due to ENOMEM.
> ABORT_TASK: Found referenced iSCSI task_tag: 33
> ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for ref_tag: 33
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> isert: isert_post_recv: ib_post_recv() failed with ret: -22
> isert: isert_post_response: ib_post_recv failed with -22
> cxgb4 0000:84:00.4: AE qpid 1026 opcode 10 status 0x1 type 1 len 0x0
> wrid.hi 0x0 wrid.lo 0x4c01
> isert: isert_qp_event_callback: QP access error (3): conn
> ffff8817ddc46000
> iSCSI Login timeout on Network Portal 10.40.40.198:3260
> INFO: task iscsi_np:14744 blocked for more than 120 seconds.
>
> Here are the QP and CQ stats before my assert for queue full:
> qhp->attr.sq_num_entries 523
> qhp->attr.rq_num_entries 129
> qhp->wq.sq.qid 1026
> qhp->wq.rq.qid 1027
> qhp->wq.sq.in_use 523 <=== SQ to the brim
> qhp->wq.sq.size 524 <=== SQ size
> qhp->wq.sq.cidx 391
> qhp->wq.sq.pidx 390
> qhp->wq.sq.wq_pidx 202
> qhp->wq.sq.wq_pidx_inc 0
> qhp->wq.sq.flush_cidx 391
> qhp->wq.rq.in_use 128
> qhp->wq.rq.size 130
> qhp->wq.rq.cidx 112
> qhp->wq.rq.pidx 110
> qhp->wq.rq.wq_pidx 240
> qhp->wq.rq.wq_pidx_inc 0
> qhp->wq.flushed 0
> chp->cq.cqid 1024
> chp->cq.size 6335
> chp->cq.cidx 4978
> chp->cq.sw_cidx 4126
> chp->cq.sw_pidx 4126
> chp->cq.sw_in_use 0
> chp->cq.cidx_inc 0
>
> As an experiment I tried increasing the ISCSI_DEF_XMIT_CMDS_MAX to 256
> instead of 128, which incase of iwarp creates SQ with size 1548, the
> issue is not seen.
> I doubt if the SQ is not sized properly incase of iWARP Or factored
> incorrectly in the rdma api for iser IOP.
>
> I am digging for root cause.
>
> Thanks for your time,
> Bharat.
>
Hey Sagi, I'm looking at isert_create_qp() and it appears to not be correctly
sizing the SQ:
...
#define ISERT_QP_MAX_REQ_DTOS (ISCSI_DEF_XMIT_CMDS_MAX + \
ISERT_MAX_TX_MISC_PDUS + \
ISERT_MAX_RX_MISC_PDUS)
...
attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS + 1;
attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS + 1;
...
I think above snipit assumes a DTO consumes exactly one WR/WQE in the SQ. But
the DTO can be broken into multiple WRs to handle REG_MRs, multiple WRITE or
READ WRs due to limits on local sge depths target sge depths, etc. Yes? Or am
I all wet? Or perhaps isert doesn't require the SQ to be the max possible
because it flow controls the DTO submissions?
Stevo
^ permalink raw reply
* Re: [PATCH rdma-core 4/4] glue/redhat/spec: build split rpm packages
From: Jarod Wilson @ 2016-10-17 18:45 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161014233904.GD16509-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Fri, Oct 14, 2016 at 05:39:04PM -0600, Jason Gunthorpe wrote:
> On Fri, Oct 14, 2016 at 03:21:36PM -0400, Jarod Wilson wrote:
>
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/iwpmd-11-1.el7.x86_64.rpm
>
> FC used to call this libiwpm for some reason..
Ah. I may need to add an Obsoletes: for that particular case then.
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libibcm-11-1.el7.x86_64.rpm
>
> Do you want to try and do as I did with debian and put the SONAME data
> into the package version for the libraries? eg
>
> libibcm1_1.0.11-1_amd64.deb
No, that's a debian-ism that Red Hat has never really followed. The soname
data is already captured in the rpm metadata.
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb3-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libcxgb4-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libhfi1-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libi40iw-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libipathverbs-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx4-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmlx5-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libmthca-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libnes-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/libocrdma-11-1.el7.x86_64.rpm
> > Wrote: /home/jwilson/rpmbuild/RPMS/x86_64/librxe-11-1.el7.x86_64.rpm
>
> Does this mean RH is not going to do a single package for the
> providers? You know another 5 or so are coming..
Putting all the providers in a single sub-package is certainly something
we could do. With rpm virtual Provides and Obsoletes, we could make the
upgrade path transparent to at least yum/dnf/rpm, but it might still
confuse people who go looking for the same package they've always had
installed, so it could be something we do in Fedora and the next RHEL
major release, rather than in a current RHEL minor update.
> Any feedback if things build on your supported arches? I'd be
> interested to see compiler log if it isn't silent.
>
> > +install -D -m0644 Documentation/{ibacm,ibsrpdm,libibcm,libibverbs,librdmacm,rxe}.md %{buildroot}/%{_docdir}/%{name}-%{version}/
> > +install -D -m0644 README.md %{buildroot}/%{_docdir}/%{name}-%{version}/
>
> I guess README should go in that patch I sent you..
D'oh, yeah, I'll mix that in here locally.
> > +%{buildroot}/%{_bindir}/ib_acme -D . -O
> > +# Fixup a multilib conflict in ibacm_opts.cfg:
> > +sed -i -e '/^# Specifies the directory of the provider libraries$/ a\
> > +# Use /usr/lib64/ibacm on 64bit, /usr/lib/libacm on 32bit.
> > +' -e 's%^\(# provider_lib_path
> > /usr/\)lib\(64\)\?/ibacm$%\1lib64/ibacm%' ibacm_opts.cfg
>
> Hum? I'm pretty sure this is basically fixed in the code now, was done here
> 5eebdb9baaaae420a4bb16e586a96807823916a0
>
> Adjusting the comment like that doesn't really make sense, the acm
> daemon has a fixed endianness and looks in a single place to load the
> plugin. If someone wants to use 32 bit plugins they have to install
> the 32 bit acm daemon, which would have the 32 bit path in the sample
> conf file...
This was just copy and paste from our existing libacm spec, might well be
something that was fixed and I just missed it.
> > +install -D -m0644 ibacm/man/*.1 %{buildroot}/%{_mandir}/man1/
> > +install -D -m0644 ibacm/man/*.7 %{buildroot}/%{_mandir}/man7/
> > +sed -e 's|@ACM_PROVIDER_DIR@|%{libdir}/%{name}/|g' -e 's|@CMAKE_INSTALL_FULL_LOCALSTATEDIR@|%{_localstatedir}|g' ibacm/man/ibacm_prov.7.in > %{buildroot}/%{_mandir}/man7/ibacm_prov.7
>
> ?? cmake is supposed to install these.. Looks like it works. What did
> you need done here to open code it?
They weren't getting installed, so I threw that hack in, meant to
actually say something about that, got lost in the shuffle. I'll
double-check if they're still not getting installed... Okay, they're
getting installed now. May have just been a transient error with an
earlier tarball, or something else I did wrong. I'll drop that bit.
--
Jarod Wilson
jarod-H+wXaHxf7aLQT0dZR+AlfA@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
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