* Re: [PATCH 2/3] IPoIB: Avoid reading an uninitialized member variable
From: Leon Romanovsky @ 2016-11-22 7:15 UTC (permalink / raw)
To: Bart Van Assche
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Erez Shitrit
In-Reply-To: <fd4a2913-545e-bf2f-e352-a47fd50a954f-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]
On Mon, Nov 21, 2016 at 10:21:41AM -0800, Bart Van Assche wrote:
> This patch avoids that Coverity reports the following:
>
> Using uninitialized value port_attr.state when calling printk
>
> Fixes: commit 94232d9ce817 ("IPoIB: Start multicast join process only on active ports")
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Except that it doesn't print the reason why ib_query_port failed,
it look good.
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Thanks
> ---
> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index 1909dd2..fddff40 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -575,8 +575,11 @@ void ipoib_mcast_join_task(struct work_struct *work)
> if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
> return;
>
> - if (ib_query_port(priv->ca, priv->port, &port_attr) ||
> - port_attr.state != IB_PORT_ACTIVE) {
> + if (ib_query_port(priv->ca, priv->port, &port_attr)) {
> + ipoib_dbg(priv, "ib_query_port() failed\n");
> + return;
> + }
> + if (port_attr.state != IB_PORT_ACTIVE) {
> ipoib_dbg(priv, "port state is not ACTIVE (state = %d) suspending join task\n",
> port_attr.state);
> return;
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] IB/multicast: Check ib_find_pkey() return value
From: Leon Romanovsky @ 2016-11-22 7:11 UTC (permalink / raw)
To: Bart Van Assche
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sean Hefty
In-Reply-To: <950a0611-4aad-9b85-8d18-3dff54db2820-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2136 bytes --]
On Mon, Nov 21, 2016 at 10:22:17AM -0800, Bart Van Assche wrote:
> This patch avoids that Coverity complains about not checking the
> ib_find_pkey() return value.
>
> Fixes: commit 547af76521b3 ("IB/multicast: Report errors on multicast groups if P_key changes")
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
> drivers/infiniband/core/multicast.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/multicast.c b/drivers/infiniband/core/multicast.c
> index e51b739..322cb67 100644
> --- a/drivers/infiniband/core/multicast.c
> +++ b/drivers/infiniband/core/multicast.c
> @@ -518,8 +518,11 @@ static void join_handler(int status, struct ib_sa_mcmember_rec *rec,
> process_join_error(group, status);
> else {
> int mgids_changed, is_mgid0;
> - ib_find_pkey(group->port->dev->device, group->port->port_num,
> - be16_to_cpu(rec->pkey), &pkey_index);
> +
> + if (ib_find_pkey(group->port->dev->device,
> + group->port->port_num, be16_to_cpu(rec->pkey),
> + &pkey_index))
> + pkey_index = MCAST_INVALID_PKEY_INDEX;
The coverity warning is valid, ib_find_pkey() can return ENOMEM
(ib_find_pkey()->ib_query_pkey()->(device->query_pkey())->mlx5_ib_query_pkey()->mlx5_query_mad_ifc_pkey()/mlx5_query_hca_vport_pkey())
I'm not sure that the error handling proposed is enough, pkey_index is initialized to
MCAST_INVALID_PKEY_INDEX at the beginning of function and in case of error it preserves
its value.
Don't you need to update process_group_error() function? It has code
flows with ib_find_pkey() without checking return value, which can be
anything too.
Thanks
>
> spin_lock_irq(&group->port->lock);
> if (group->state == MCAST_BUSY &&
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Vishwanathapura, Niranjana @ 2016-11-22 1:53 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Dennis Dalessandro
In-Reply-To: <20161121233118.GA31132-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
On Mon, Nov 21, 2016 at 04:31:18PM -0700, Jason Gunthorpe wrote:
>> >>>>+ ida_init(&hfi_vnic_ctrl_ida);
>> >>>>+ idr_init(&hfi_vnic_idr);
>> >>>>+
>> >>>>+ rc = bus_register(&hfi_vnic_bus);
>> >>>
>> >>>Why on earth do we need this? Didn't I give you enough grief for the
>> >>>psm stuff and now you want to create an entire subystem hidden away!?
>> >>>
>> >>>Use some netlink scheme to control your vnic like the rest of the net
>> >>>stack..
>> >>>
>> >>
>> >>The hfi_vnic_bus is only abstracting the HW independent functionality (like
>> >>Ethernet interface, encapsulation, IB MAD interface etc) with the HW
>> >>dependent functionality (sending/receiving packets on the wire).
>> >>Thus providing a cleaner interface between HW independent hfi_vnic Ethernet
>> >>and Control drivers and the HW dependent HFI1 driver.
>> >
>> >That doesn't explain anything, sound like you don't need it so get rid
>> >of it.
>
>> >>There is no other User interface here other than the standard Ethernet
>> >>interface through network stack.
>> >
>> >Good, then this isn't needed, because it doesn't provide a user interface.
>> >
>>
>> Can you explain what exactly you are asking to get rid of here and why?
>
>Get rid of the bus_register/etc as drivers do not get to call this.
>
There are many example drivers in kernel which are using bus_register() in
an initcall.
We could add a custom Interface between HFI1 driver and hfi_vnic drivers
without involving a bus.
But using the existing bus model gave a lot of in-built flexibility in
decoupling devices from the drivers.
Niranjana
>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: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Jason Gunthorpe @ 2016-11-21 23:31 UTC (permalink / raw)
To: Vishwanathapura, Niranjana
Cc: Doug Ledford, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Dennis Dalessandro
In-Reply-To: <20161121232629.GA67988-wPcXA7LoDC+1XWohqUldA0EOCMrvLtNR@public.gmane.org>
On Mon, Nov 21, 2016 at 03:26:29PM -0800, Vishwanathapura, Niranjana wrote:
> I did not see any example IB mad agent outside drivers/inifiniband
> folder.
You can be the first.
> I did see some netdev drivers outside the net/ folder (like ipoib and
> drivers/infiniband/hw/nes/).
It is very rare, and arguably ipoib was a mistake..
> The hfi_vnic Ethernet driver is entirely a soft driver without any HW access.
> Also, any interface changes between hfi_vnic and the HFI driver makes it
> difficult to manage if they are under two subsystems/maintainers.
We manage this already for all the rocee drivers that are based on
netdev drivers, you will be fine.
> We have 'rdmavt' and 'soft-roce' drivers under drivers/infiniband/sw/ folder
> which are soft drivers similar to hfi_vnic. So we decided to put
> 'hif_vnic'
As I said those are HCA drivers & support, not just random 'software'.
> >>>>+ ida_init(&hfi_vnic_ctrl_ida);
> >>>>+ idr_init(&hfi_vnic_idr);
> >>>>+
> >>>>+ rc = bus_register(&hfi_vnic_bus);
> >>>
> >>>Why on earth do we need this? Didn't I give you enough grief for the
> >>>psm stuff and now you want to create an entire subystem hidden away!?
> >>>
> >>>Use some netlink scheme to control your vnic like the rest of the net
> >>>stack..
> >>>
> >>
> >>The hfi_vnic_bus is only abstracting the HW independent functionality (like
> >>Ethernet interface, encapsulation, IB MAD interface etc) with the HW
> >>dependent functionality (sending/receiving packets on the wire).
> >>Thus providing a cleaner interface between HW independent hfi_vnic Ethernet
> >>and Control drivers and the HW dependent HFI1 driver.
> >
> >That doesn't explain anything, sound like you don't need it so get rid
> >of it.
> >>There is no other User interface here other than the standard Ethernet
> >>interface through network stack.
> >
> >Good, then this isn't needed, because it doesn't provide a user interface.
> >
>
> Can you explain what exactly you are asking to get rid of here and why?
Get rid of the bus_register/etc as drivers do not get to call this.
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: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Vishwanathapura, Niranjana @ 2016-11-21 23:26 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma, netdev, Dennis Dalessandro
In-Reply-To: <20161121213930.GA30111@obsidianresearch.com>
On Mon, Nov 21, 2016 at 02:39:30PM -0700, Jason Gunthorpe wrote:
>On Mon, Nov 21, 2016 at 01:30:17PM -0800, Vishwanathapura, Niranjana wrote:
>> On Sat, Nov 19, 2016 at 12:04:45PM -0700, Jason Gunthorpe wrote:
>> >On Fri, Nov 18, 2016 at 02:42:10PM -0800, Vishwanathapura, Niranjana wrote:
>> >>+HFI-VNIC DRIVER
>> >>+M: Dennis Dalessandro <dennis.dalessandro@intel.com>
>> >>+M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> >>+L: linux-rdma@vger.kernel.org
>> >>+S: Supported
>> >>+F: drivers/infiniband/sw/intel/vnic
>> >
>> >This is either a net driver or a ULP, no idea why it should go in this
>> >directory!?
>> >
>> >It sounds like an ethernet driver, so you should probably put it
>> >there...
>> >
>>
>> The hfi_vnic is an Ethernet driver. It is similar to ULP like ipoib, but
>> instead it is Ethernet over Omni-path here.
>> The VNIC Ethernet (hfi_vnic) driver encapsulates Ethernet packets in an
>> Omni-path header.
>> The hfi_vnic Ethernet driver do not access the HW. It interfaces with HFI1
>> driver which sends/receives Omni-Path encapsulated Ethernet frames from HW.
>> Also, the VNIC control path driver (VEMA) is an IB MAD agent which should be
>> under drivers/infiniband/.. .
>> Putting the VNIC Ethernet driver and the VNIC control driver together under
>> a single module (hfi_vnic.ko) provided a simpler interface between them.
>>
>> So, we have put the driver under drivers/infiniband/sw/intel for two reasons:
>> a) We have VNIC control driver (VEMA) which is an IB mad agent.
>> b) hfi_vnic Ethernet driver is dependent on HFI1 driver for sending/receving
>> Omni-path encapsulated Ethernet packets from HW.
>
>Sounds like this driver belongs under net/ someplace to me.
>
>NAK on drivers/infiniband/sw/ at least - that dir is only for HCA
>drivers.
>
I did not see any example IB mad agent outside drivers/inifiniband folder.
I did see some netdev drivers outside the net/ folder (like ipoib and
drivers/infiniband/hw/nes/).
The hfi_vnic Ethernet driver is entirely a soft driver without any HW access.
Also, any interface changes between hfi_vnic and the HFI driver makes it
difficult to manage if they are under two subsystems/maintainers.
So drivers/infiniband is probably more approriate for this driver.
We have 'rdmavt' and 'soft-roce' drivers under drivers/infiniband/sw/ folder
which are soft drivers similar to hfi_vnic. So we decided to put 'hif_vnic'
under there.
Other places under drivers/infiniband where we can put this driver are,
drivers/infiniband/ulp/hfi_vnic
drivers/infiniband/hw/hfi_vnic
>> >>+/* hfi_vnic_bus_init - initialize the hfi vnic bus drvier */
>> >>+static int hfi_vnic_bus_init(void)
>> >>+{
>> >>+ int rc;
>> >>+
>> >>+ ida_init(&hfi_vnic_ctrl_ida);
>> >>+ idr_init(&hfi_vnic_idr);
>> >>+
>> >>+ rc = bus_register(&hfi_vnic_bus);
>> >
>> >Why on earth do we need this? Didn't I give you enough grief for the
>> >psm stuff and now you want to create an entire subystem hidden away!?
>> >
>> >Use some netlink scheme to control your vnic like the rest of the net
>> >stack..
>> >
>>
>> The hfi_vnic_bus is only abstracting the HW independent functionality (like
>> Ethernet interface, encapsulation, IB MAD interface etc) with the HW
>> dependent functionality (sending/receiving packets on the wire).
>> Thus providing a cleaner interface between HW independent hfi_vnic Ethernet
>> and Control drivers and the HW dependent HFI1 driver.
>
>That doesn't explain anything, sound like you don't need it so get rid
>of it.
>
>> There is no other User interface here other than the standard Ethernet
>> interface through network stack.
>
>Good, then this isn't needed, because it doesn't provide a user interface.
>
Can you explain what exactly you are asking to get rid of here and why?
>> #ls /sys/bus/hfi_vnic_bus/devices/
>> hfi_vnic_ctrl_00 /* control device for HFI instance 0 */
>> hfi_vnic_00.01.00 /* first VNIC port on HFI instance 0, port 1 */
>
>Jason
^ permalink raw reply
* Re: [PATCH v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait
From: Arnd Bergmann @ 2016-11-21 22:51 UTC (permalink / raw)
To: Linus Torvalds
Cc: Binoy Jayan, linux-rdma@vger.kernel.org, Mustafa Ismail, Lijun Ou,
Nicholas Bellinger, Leon Romanovsky, target-devel,
Tatyana E Nikolova, Doug Ledford, Jenny Derzhavetz, Sagi Grimberg,
Sean Hefty, Faisal Latif, Hal Rosenstock,
Linux Kernel Mailing List, Wei Hu(Xavier), Mark Brown, Mark Bloch,
Steve
In-Reply-To: <CA+55aFyd7d+C39ipC5XuZg1gJjJ+HFSgcPQU7Xb26Fsh6Z1_0A@mail.gmail.com>
On Monday, November 21, 2016 9:57:51 AM CET Linus Torvalds wrote:
>
> - semaphores are "old-fashioned mutexes". A mutex is better than a
> semaphore, but a semaphore is better than just about all the other
> alternatives. There's nothing _wrong_ with using a semaphore per se.
>
> In this case, either use a semaphore or a mutex. If you are doing
> mutual exclusion, those are really the only two acceptable sleeping
> models.
The main problem with semaphores is that they are slowly spreading
into areas that really should be mutexes or completions. A couple
of years ago, we had only around 30 semaphores left in the kernel
and while a lot of those have been removed in the meantime, over
100 new ones have come in, the majority of them in the category
that can be trivially converted to a mutex or semaphore.
This in turn is not much of a problem, except to a certain degree
for preempt-rt users.
I suggested to Binoy that he could look into replacing the existing
semaphores one subsystem at a time under the assumption that we
could find a relatively easy alternative for every one of them
and then remove the implementation completely.
Christoph's suggestion is probably more productive here: let's
remove the ones that are obviously wrong or inferior, and only
once they have been taken care of we can look into whether it's
worth doing something about the rest or not.
Arnd
^ permalink raw reply
* [PATCH 5/5] IB/srp: Make writing into the add_target sysfs attribute interruptible
From: Bart Van Assche @ 2016-11-21 21:58 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <ba32b251-135f-c57d-44cc-658ae35a49ca-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Avoid that shutdown of srp_daemon is delayed if add_target_mutex is
held by another process.
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bb9d73d..8ddc071 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3299,7 +3299,9 @@ static ssize_t srp_create_target(struct device *dev,
*/
scsi_host_get(target->scsi_host);
- mutex_lock(&host->add_target_mutex);
+ ret = mutex_lock_interruptible(&host->add_target_mutex);
+ if (ret < 0)
+ goto put;
ret = srp_parse_options(buf, target);
if (ret)
@@ -3455,6 +3457,7 @@ static ssize_t srp_create_target(struct device *dev,
out:
mutex_unlock(&host->add_target_mutex);
+put:
scsi_host_put(target->scsi_host);
if (ret < 0)
scsi_host_put(target->scsi_host);
--
2.10.2
--
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/5] IB/srp: Make mapping failures easier to debug
From: Bart Van Assche @ 2016-11-21 21:57 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <ba32b251-135f-c57d-44cc-658ae35a49ca-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Make it easier to figure out what is going on if memory mapping
fails because more memory regions than mr_per_cmd are needed.
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 81cb27f..bb9d73d 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1274,8 +1274,12 @@ static int srp_map_finish_fmr(struct srp_map_state *state,
struct ib_pool_fmr *fmr;
u64 io_addr = 0;
- if (state->fmr.next >= state->fmr.end)
+ if (state->fmr.next >= state->fmr.end) {
+ shost_printk(KERN_ERR, ch->target->scsi_host,
+ PFX "Out of MRs (mr_per_cmd = %d)\n",
+ ch->target->mr_per_cmd);
return -ENOMEM;
+ }
WARN_ON_ONCE(!dev->use_fmr);
@@ -1331,8 +1335,12 @@ static int srp_map_finish_fr(struct srp_map_state *state,
u32 rkey;
int n, err;
- if (state->fr.next >= state->fr.end)
+ if (state->fr.next >= state->fr.end) {
+ shost_printk(KERN_ERR, ch->target->scsi_host,
+ PFX "Out of MRs (mr_per_cmd = %d)\n",
+ ch->target->mr_per_cmd);
return -ENOMEM;
+ }
WARN_ON_ONCE(!dev->use_fast_reg);
--
2.10.2
--
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 3/5] IB/srp: Make login failures easier to debug
From: Bart Van Assche @ 2016-11-21 21:57 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <ba32b251-135f-c57d-44cc-658ae35a49ca-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
If login fails because memory region allocation failed it can be
hard to figure out what happened. Make it easier to figure out
why login failed by logging a message if ib_alloc_mr() fails.
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index c216c6e..81cb27f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -389,6 +389,9 @@ static struct srp_fr_pool *srp_create_fr_pool(struct ib_device *device,
max_page_list_len);
if (IS_ERR(mr)) {
ret = PTR_ERR(mr);
+ if (ret == -ENOMEM)
+ pr_info("%s: ib_alloc_mr() failed. Try to reduce max_cmd_per_lun, max_sect or ch_count\n",
+ dev_name(&device->dev));
goto destroy_pool;
}
d->mr = mr;
--
2.10.2
--
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/5] IB/srp: Introduce a local variable in srp_add_one()
From: Bart Van Assche @ 2016-11-21 21:57 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <ba32b251-135f-c57d-44cc-658ae35a49ca-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
This patch makes the srp_add_one() code more compact and does not
change any functionality.
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8bb720c..c216c6e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -3527,6 +3527,7 @@ static struct srp_host *srp_add_port(struct srp_device *device, u8 port)
static void srp_add_one(struct ib_device *device)
{
struct srp_device *srp_dev;
+ struct ib_device_attr *attr = &device->attrs;
struct srp_host *host;
int mr_page_shift, p;
u64 max_pages_per_mr;
@@ -3541,25 +3542,25 @@ static void srp_add_one(struct ib_device *device)
* minimum of 4096 bytes. We're unlikely to build large sglists
* out of smaller entries.
*/
- mr_page_shift = max(12, ffs(device->attrs.page_size_cap) - 1);
+ mr_page_shift = max(12, ffs(attr->page_size_cap) - 1);
srp_dev->mr_page_size = 1 << mr_page_shift;
srp_dev->mr_page_mask = ~((u64) srp_dev->mr_page_size - 1);
- max_pages_per_mr = device->attrs.max_mr_size;
+ max_pages_per_mr = attr->max_mr_size;
do_div(max_pages_per_mr, srp_dev->mr_page_size);
pr_debug("%s: %llu / %u = %llu <> %u\n", __func__,
- device->attrs.max_mr_size, srp_dev->mr_page_size,
+ attr->max_mr_size, srp_dev->mr_page_size,
max_pages_per_mr, SRP_MAX_PAGES_PER_MR);
srp_dev->max_pages_per_mr = min_t(u64, SRP_MAX_PAGES_PER_MR,
max_pages_per_mr);
srp_dev->has_fmr = (device->alloc_fmr && device->dealloc_fmr &&
device->map_phys_fmr && device->unmap_fmr);
- srp_dev->has_fr = (device->attrs.device_cap_flags &
+ srp_dev->has_fr = (attr->device_cap_flags &
IB_DEVICE_MEM_MGT_EXTENSIONS);
if (!never_register && !srp_dev->has_fmr && !srp_dev->has_fr) {
dev_warn(&device->dev, "neither FMR nor FR is supported\n");
} else if (!never_register &&
- device->attrs.max_mr_size >= 2 * srp_dev->mr_page_size) {
+ attr->max_mr_size >= 2 * srp_dev->mr_page_size) {
srp_dev->use_fast_reg = (srp_dev->has_fr &&
(!srp_dev->has_fmr || prefer_fr));
srp_dev->use_fmr = !srp_dev->use_fast_reg && srp_dev->has_fmr;
@@ -3572,13 +3573,13 @@ static void srp_add_one(struct ib_device *device)
if (srp_dev->use_fast_reg) {
srp_dev->max_pages_per_mr =
min_t(u32, srp_dev->max_pages_per_mr,
- device->attrs.max_fast_reg_page_list_len);
+ attr->max_fast_reg_page_list_len);
}
srp_dev->mr_max_size = srp_dev->mr_page_size *
srp_dev->max_pages_per_mr;
pr_debug("%s: mr_page_shift = %d, device->max_mr_size = %#llx, device->max_fast_reg_page_list_len = %u, max_pages_per_mr = %d, mr_max_size = %#x\n",
- device->name, mr_page_shift, device->attrs.max_mr_size,
- device->attrs.max_fast_reg_page_list_len,
+ device->name, mr_page_shift, attr->max_mr_size,
+ attr->max_fast_reg_page_list_len,
srp_dev->max_pages_per_mr, srp_dev->mr_max_size);
INIT_LIST_HEAD(&srp_dev->dev_list);
--
2.10.2
--
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 1/5] IB/srp: Fix CONFIG_DYNAMIC_DEBUG=n build
From: Bart Van Assche @ 2016-11-21 21:56 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <ba32b251-135f-c57d-44cc-658ae35a49ca-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Avoid that the kernel build fails as follows if dynamic debug support
is disabled:
drivers/infiniband/ulp/srp/ib_srp.c:2272:3: error: implicit declaration of function 'DEFINE_DYNAMIC_DEBUG_METADATA'
drivers/infiniband/ulp/srp/ib_srp.c:2272:33: error: 'ddm' undeclared (first use in this function)
drivers/infiniband/ulp/srp/ib_srp.c:2275:39: error: '_DPRINTK_FLAGS_PRINT' undeclared (first use in this function)
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
---
drivers/infiniband/ulp/srp/ib_srp.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index d980fb4..8bb720c 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -64,6 +64,11 @@ MODULE_LICENSE("Dual BSD/GPL");
MODULE_VERSION(DRV_VERSION);
MODULE_INFO(release_date, DRV_RELDATE);
+#if !defined(CONFIG_DYNAMIC_DEBUG)
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)
+#define DYNAMIC_DEBUG_BRANCH(descriptor) false
+#endif
+
static unsigned int srp_sg_tablesize;
static unsigned int cmd_sg_entries;
static unsigned int indirect_sg_entries;
@@ -1556,7 +1561,6 @@ static int srp_map_idb(struct srp_rdma_ch *ch, struct srp_request *req,
return 0;
}
-#if defined(DYNAMIC_DATA_DEBUG)
static void srp_check_mapping(struct srp_map_state *state,
struct srp_rdma_ch *ch, struct srp_request *req,
struct scatterlist *scat, int count)
@@ -1580,7 +1584,6 @@ static void srp_check_mapping(struct srp_map_state *state,
scsi_bufflen(req->scmnd), desc_len, mr_len,
state->ndesc, state->nmdesc);
}
-#endif
/**
* srp_map_data() - map SCSI data buffer onto an SRP request
@@ -1669,14 +1672,12 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch,
if (ret < 0)
goto unmap;
-#if defined(DYNAMIC_DEBUG)
{
DEFINE_DYNAMIC_DEBUG_METADATA(ddm,
"Memory mapping consistency check");
- if (unlikely(ddm.flags & _DPRINTK_FLAGS_PRINT))
+ if (DYNAMIC_DEBUG_BRANCH(ddm))
srp_check_mapping(&state, ch, req, scat, count);
}
-#endif
/* We've mapped the request, now pull as much of the indirect
* descriptor table as we can into the command buffer. If this
--
2.10.2
--
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 0/5] IB/srp patches for kernel v4.10
From: Bart Van Assche @ 2016-11-21 21:56 UTC (permalink / raw)
To: Doug Ledford
Cc: Max Gurtovoy, Christoph Hellwig,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hello Doug,
The patches in this series are what I came up with while testing the
v4.9-rc5 IB/srp driver. Please consider these patches for inclusion in
kernel v4.10. The individual patches in this series are:
0001-IB-srp-Fix-CONFIG_DYNAMIC_DEBUG-n-build.patch
0002-IB-srp-Introduce-a-local-variable-in-srp_add_one.patch
0003-IB-srp-Make-login-failures-easier-to-debug.patch
0004-IB-srp-Make-mapping-failures-easier-to-debug.patch
0005-IB-srp-Make-writing-into-the-add_target-sysfs-attrib.patch
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: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Jason Gunthorpe @ 2016-11-21 21:39 UTC (permalink / raw)
To: Vishwanathapura, Niranjana
Cc: Doug Ledford, linux-rdma, netdev, Dennis Dalessandro
In-Reply-To: <20161121213017.GB67872@knc-06.sc.intel.com>
On Mon, Nov 21, 2016 at 01:30:17PM -0800, Vishwanathapura, Niranjana wrote:
> On Sat, Nov 19, 2016 at 12:04:45PM -0700, Jason Gunthorpe wrote:
> >On Fri, Nov 18, 2016 at 02:42:10PM -0800, Vishwanathapura, Niranjana wrote:
> >>+HFI-VNIC DRIVER
> >>+M: Dennis Dalessandro <dennis.dalessandro@intel.com>
> >>+M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> >>+L: linux-rdma@vger.kernel.org
> >>+S: Supported
> >>+F: drivers/infiniband/sw/intel/vnic
> >
> >This is either a net driver or a ULP, no idea why it should go in this
> >directory!?
> >
> >It sounds like an ethernet driver, so you should probably put it
> >there...
> >
>
> The hfi_vnic is an Ethernet driver. It is similar to ULP like ipoib, but
> instead it is Ethernet over Omni-path here.
> The VNIC Ethernet (hfi_vnic) driver encapsulates Ethernet packets in an
> Omni-path header.
> The hfi_vnic Ethernet driver do not access the HW. It interfaces with HFI1
> driver which sends/receives Omni-Path encapsulated Ethernet frames from HW.
> Also, the VNIC control path driver (VEMA) is an IB MAD agent which should be
> under drivers/infiniband/.. .
> Putting the VNIC Ethernet driver and the VNIC control driver together under
> a single module (hfi_vnic.ko) provided a simpler interface between them.
>
> So, we have put the driver under drivers/infiniband/sw/intel for two reasons:
> a) We have VNIC control driver (VEMA) which is an IB mad agent.
> b) hfi_vnic Ethernet driver is dependent on HFI1 driver for sending/receving
> Omni-path encapsulated Ethernet packets from HW.
Sounds like this driver belongs under net/ someplace to me.
NAK on drivers/infiniband/sw/ at least - that dir is only for HCA
drivers.
> >>+/* hfi_vnic_bus_init - initialize the hfi vnic bus drvier */
> >>+static int hfi_vnic_bus_init(void)
> >>+{
> >>+ int rc;
> >>+
> >>+ ida_init(&hfi_vnic_ctrl_ida);
> >>+ idr_init(&hfi_vnic_idr);
> >>+
> >>+ rc = bus_register(&hfi_vnic_bus);
> >
> >Why on earth do we need this? Didn't I give you enough grief for the
> >psm stuff and now you want to create an entire subystem hidden away!?
> >
> >Use some netlink scheme to control your vnic like the rest of the net
> >stack..
> >
>
> The hfi_vnic_bus is only abstracting the HW independent functionality (like
> Ethernet interface, encapsulation, IB MAD interface etc) with the HW
> dependent functionality (sending/receiving packets on the wire).
> Thus providing a cleaner interface between HW independent hfi_vnic Ethernet
> and Control drivers and the HW dependent HFI1 driver.
That doesn't explain anything, sound like you don't need it so get rid
of it.
> There is no other User interface here other than the standard Ethernet
> interface through network stack.
Good, then this isn't needed, because it doesn't provide a user interface.
> #ls /sys/bus/hfi_vnic_bus/devices/
> hfi_vnic_ctrl_00 /* control device for HFI instance 0 */
> hfi_vnic_00.01.00 /* first VNIC port on HFI instance 0, port 1 */
Jason
^ permalink raw reply
* Re: [RFC 02/10] IB/hfi-vnic: Virtual Network Interface Controller (VNIC) Bus driver
From: Vishwanathapura, Niranjana @ 2016-11-21 21:30 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma, netdev, Dennis Dalessandro
In-Reply-To: <20161119190445.GG22775@obsidianresearch.com>
On Sat, Nov 19, 2016 at 12:04:45PM -0700, Jason Gunthorpe wrote:
>On Fri, Nov 18, 2016 at 02:42:10PM -0800, Vishwanathapura, Niranjana wrote:
>> +HFI-VNIC DRIVER
>> +M: Dennis Dalessandro <dennis.dalessandro@intel.com>
>> +M: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
>> +L: linux-rdma@vger.kernel.org
>> +S: Supported
>> +F: drivers/infiniband/sw/intel/vnic
>
>This is either a net driver or a ULP, no idea why it should go in this
>directory!?
>
>It sounds like an ethernet driver, so you should probably put it
>there...
>
The hfi_vnic is an Ethernet driver. It is similar to ULP like ipoib, but
instead it is Ethernet over Omni-path here.
The VNIC Ethernet (hfi_vnic) driver encapsulates Ethernet packets in an
Omni-path header.
The hfi_vnic Ethernet driver do not access the HW. It interfaces with HFI1
driver which sends/receives Omni-Path encapsulated Ethernet frames from HW.
Also, the VNIC control path driver (VEMA) is an IB MAD agent which should be
under drivers/infiniband/.. .
Putting the VNIC Ethernet driver and the VNIC control driver together under a
single module (hfi_vnic.ko) provided a simpler interface between them.
So, we have put the driver under drivers/infiniband/sw/intel for two reasons:
a) We have VNIC control driver (VEMA) which is an IB mad agent.
b) hfi_vnic Ethernet driver is dependent on HFI1 driver for sending/receving
Omni-path encapsulated Ethernet packets from HW.
>> +/* hfi_vnic_bus_init - initialize the hfi vnic bus drvier */
>> +static int hfi_vnic_bus_init(void)
>> +{
>> + int rc;
>> +
>> + ida_init(&hfi_vnic_ctrl_ida);
>> + idr_init(&hfi_vnic_idr);
>> +
>> + rc = bus_register(&hfi_vnic_bus);
>
>Why on earth do we need this? Didn't I give you enough grief for the
>psm stuff and now you want to create an entire subystem hidden away!?
>
>Use some netlink scheme to control your vnic like the rest of the net
>stack..
>
The hfi_vnic_bus is only abstracting the HW independent functionality (like
Ethernet interface, encapsulation, IB MAD interface etc) with the HW dependent
functionality (sending/receiving packets on the wire).
Thus providing a cleaner interface between HW independent hfi_vnic Ethernet and
Control drivers and the HW dependent HFI1 driver.
There is no other User interface here other than the standard Ethernet
interface through network stack.
HFI1 driver creates VNIC devices on the hfi_vnic_bus as below and the hfi_vnic
Ethernet and Control drivers drive them.
#ls /sys/bus/hfi_vnic_bus/devices/
hfi_vnic_ctrl_00 /* control device for HFI instance 0 */
hfi_vnic_00.01.00 /* first VNIC port on HFI instance 0, port 1 */
hfi_vnic_00.01.01 /* second VNIC port on HFI instance 0, port 1 */
The design is as shown in the below diagram.
+-------------------+ +----------------------+
| | | Linux |
| IB MAD | | Network |
| | | Stack |
+-------------------+ +----------------------+
| |
| |
+--------------------------------------------+
| |
| HFI VNIC Module |
| (HFI VNIC Netdev and EMA drivers) |
| (HW independent) |
+--------------------------------------------+
|
|
+--------------------------------------------+
| HFI VNIC Bus |
+--------------------------------------------+
|
|
+--------------------------------------------+
| |
| HFI1 Driver with VNIC support |
| (HW dependent) |
+--------------------------------------------+
Niranjana
>Jason
^ permalink raw reply
* Re: [PATCH 1/3] IB/mad: Fix an array index check
From: Hal Rosenstock @ 2016-11-21 21:17 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty
In-Reply-To: <ab9e8563-160b-a9de-08b3-b09bdd52fbad-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Hi Bart,
On 11/21/2016 3:52 PM, Bart Van Assche wrote:
> Hello Hal,
>
> I think such a check is already present in ib_register_mad_agent():
>
> if (mad_reg_req->mgmt_class >= MAX_MGMT_CLASS) {
You're right.
-- Hal
--
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 1/3] IB/mad: Fix an array index check
From: Bart Van Assche @ 2016-11-21 20:52 UTC (permalink / raw)
To: Hal Rosenstock, Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty
In-Reply-To: <d5924c69-c052-84d3-06e3-b74ed8eb58d3-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>
On 11/21/2016 12:17 PM, Hal Rosenstock wrote:
> I think there is also similar thing in missing check in ib_register_mad_agent where:
>
> /*
> * Make sure MAD registration (if supplied)
> * is non overlapping with any existing ones
> */
> if (mad_reg_req) {
> mgmt_class = convert_mgmt_class(mad_reg_req->mgmt_class);
> if (!is_vendor_class(mgmt_class)) {
> class = port_priv->version[mad_reg_req->
> mgmt_class_version].class;
> if (class) {
> method = class->method_table[mgmt_class];
>
> so here the class' method_table is also accessed without checking mgmt_class for range violation, right ?
Hello Hal,
I think such a check is already present in ib_register_mad_agent():
if (mad_reg_req->mgmt_class >= MAX_MGMT_CLASS) {
/*
* IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE is the only
* one in this range currently allowed
*/
if (mad_reg_req->mgmt_class !=
IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE) {
dev_notice(&device->dev,
"%s: Invalid Mgmt Class 0x%x\n",
__func__, mad_reg_req->mgmt_class);
goto error1;
}
} [ ... ]
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
* Enabling peer to peer device transactions for PCIe devices
From: Deucher, Alexander @ 2016-11-21 20:36 UTC (permalink / raw)
To: 'linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org',
'linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org',
'linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org',
'Linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org',
'dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org',
'linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org'
Cc: Bridgman, John, Kuehling, Felix, Sagalovitch, Serguei,
Blinzer, Paul, Koenig, Christian, Suthikulpanit, Suravee,
Sander, Ben
This is certainly not the first time this has been brought up, but I'd like to try and get some consensus on the best way to move this forward. Allowing devices to talk directly improves performance and reduces latency by avoiding the use of staging buffers in system memory. Also in cases where both devices are behind a switch, it avoids the CPU entirely. Most current APIs (DirectGMA, PeerDirect, CUDA, HSA) that deal with this are pointer based. Ideally we'd be able to take a CPU virtual address and be able to get to a physical address taking into account IOMMUs, etc. Having struct pages for the memory would allow it to work more generally and wouldn't require as much explicit support in drivers that wanted to use it.
Some use cases:
1. Storage devices streaming directly to GPU device memory
2. GPU device memory to GPU device memory streaming
3. DVB/V4L/SDI devices streaming directly to GPU device memory
4. DVB/V4L/SDI devices streaming directly to storage devices
Here is a relatively simple example of how this could work for testing. This is obviously not a complete solution.
- Device memory will be registered with Linux memory sub-system by created corresponding struct page structures for device memory
- get_user_pages_fast() will return corresponding struct pages when CPU address points to the device memory
- put_page() will deal with struct pages for device memory
Previously proposed solutions and related proposals:
1.P2P DMA
DMA-API/PCI map_peer_resource support for peer-to-peer (http://www.spinics.net/lists/linux-pci/msg44560.html)
Pros: Low impact, already largely reviewed.
Cons: requires explicit support in all drivers that want to support it, doesn't handle S/G in device memory.
2. ZONE_DEVICE IO
Direct I/O and DMA for persistent memory (https://lwn.net/Articles/672457/)
Add support for ZONE_DEVICE IO memory with struct pages. (https://patchwork.kernel.org/patch/8583221/)
Pro: Doesn't waste system memory for ZONE metadata
Cons: CPU access to ZONE metadata slow, may be lost, corrupted on device reset.
3. DMA-BUF
RDMA subsystem DMA-BUF support (http://www.spinics.net/lists/linux-rdma/msg38748.html)
Pros: uses existing dma-buf interface
Cons: dma-buf is handle based, requires explicit dma-buf support in drivers.
4. iopmem
iopmem : A block device for PCIe memory (https://lwn.net/Articles/703895/)
5. HMM
Heterogeneous Memory Management (http://lkml.iu.edu/hypermail/linux/kernel/1611.2/02473.html)
6. Some new mmap-like interface that takes a userptr and a length and returns a dma-buf and offset?
Alex
^ permalink raw reply
* RE: [PATCH for-next 03/11] IB/hns: Optimize the logic of allocating memory using APIs
From: Salil Mehta @ 2016-11-21 20:20 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, Huwei (Xavier),
oulijun, mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm,
Zhangping (ZP)
In-Reply-To: <20161121171423.GA23083-2ukJVAZIZ/Y@public.gmane.org>
> -----Original Message-----
> From: netdev-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:netdev-
> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Leon Romanovsky
> Sent: Monday, November 21, 2016 5:14 PM
> To: Salil Mehta
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Huwei (Xavier); oulijun;
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm;
> Zhangping (ZP)
> Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of
> allocating memory using APIs
>
> On Mon, Nov 21, 2016 at 04:12:38PM +0000, Salil Mehta wrote:
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > Sent: Wednesday, November 16, 2016 8:36 AM
> > > To: Salil Mehta
> > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Huwei (Xavier); oulijun;
> > > mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm;
> > > Zhangping (ZP)
> > > Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic of
> > > allocating memory using APIs
> > >
> > > On Tue, Nov 15, 2016 at 03:52:46PM +0000, Salil Mehta wrote:
> > > > > -----Original Message-----
> > > > > From: Leon Romanovsky [mailto:leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> > > > > Sent: Wednesday, November 09, 2016 7:22 AM
> > > > > To: Salil Mehta
> > > > > Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Huwei (Xavier); oulijun;
> > > > > mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > > > netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm;
> > > > > Zhangping (ZP)
> > > > > Subject: Re: [PATCH for-next 03/11] IB/hns: Optimize the logic
> of
> > > > > allocating memory using APIs
> > > > >
> > > > > On Fri, Nov 04, 2016 at 04:36:25PM +0000, Salil Mehta wrote:
> > > > > > From: "Wei Hu (Xavier)" <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > >
> > > > > > This patch modified the logic of allocating memory using APIs
> in
> > > > > > hns RoCE driver. We used kcalloc instead of kmalloc_array and
> > > > > > bitmap_zero. And When kcalloc failed, call vzalloc to alloc
> > > > > > memory.
> > > > > >
> > > > > > Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > > Signed-off-by: Ping Zhang <zhangping5-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > > Signed-off-by: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > > ---
> > > > > > drivers/infiniband/hw/hns/hns_roce_mr.c | 15 ++++++++-----
> --
> > > > > > 1 file changed, 8 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > b/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > index fb87883..d3dfb5f 100644
> > > > > > --- a/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > +++ b/drivers/infiniband/hw/hns/hns_roce_mr.c
> > > > > > @@ -137,11 +137,12 @@ static int hns_roce_buddy_init(struct
> > > > > hns_roce_buddy *buddy, int max_order)
> > > > > >
> > > > > > for (i = 0; i <= buddy->max_order; ++i) {
> > > > > > s = BITS_TO_LONGS(1 << (buddy->max_order - i));
> > > > > > - buddy->bits[i] = kmalloc_array(s, sizeof(long),
> > > > > GFP_KERNEL);
> > > > > > - if (!buddy->bits[i])
> > > > > > - goto err_out_free;
> > > > > > -
> > > > > > - bitmap_zero(buddy->bits[i], 1 << (buddy->max_order -
> > > i));
> > > > > > + buddy->bits[i] = kcalloc(s, sizeof(long),
> > > GFP_KERNEL);
> > > > > > + if (!buddy->bits[i]) {
> > > > > > + buddy->bits[i] = vzalloc(s * sizeof(long));
> > > > >
> > > > > I wonder, why don't you use directly vzalloc instead of kcalloc
> > > > > fallback?
> > > > As we know we will have physical contiguous pages if the kcalloc
> > > > call succeeds. This will give us a chance to have better
> performance
> > > > over the allocations which are just virtually contiguous through
> the
> > > > function vzalloc(). Therefore, later has only been used as a
> fallback
> > > > when our memory request cannot be entertained through kcalloc.
> > > >
> > > > Are you suggesting that there will not be much performance
> penalty
> > > > if we use just vzalloc ?
> > >
> > > Not exactly,
> > > I asked it, because we have similar code in our drivers and this
> > > construction looks strange to me.
> > >
> > > 1. If performance is critical, we will use kmalloc.
> > > 2. If performance is not critical, we will use vmalloc.
> > >
> > > But in this case, such construction shows me that we can live with
> > > vmalloc performance and kmalloc allocation are not really needed.
> > >
> > > In your specific case, I'm not sure that kcalloc will ever fail.
> > Performance is definitely critical here. Though, I agree this is bit
> > unusual way of memory allocation. In actual, we were encountering
> > memory alloc failures using kmalloc (if you see allocation amount
> > is on the higher side and is exponential) so we ended up using
> > vmalloc as fall back - It is very naïve allocation scheme.
>
> I understand it, we did the same, see our mlx5_vzalloc call.
> BTW, we used __GFP_NOWARN flag, which you should consider to use
> in your case too.
Ok. Will add this flag and refloat patch V3.
Thanks
>
> >
> > Maybe we need to rethink this allocation scheme part? Also, I can
> pull
> > back this particular patch for now or just live with vzalloc() till
> > we figure out proper solution to this?
>
> It is up to you, I don't think that you should drop it, AFAIK, there is
> no other proper solution.
Ok we will live with it for now and later maybe we can see how we can optimize
pre-allocation of physically contiguous memory.
Thanks for your suggestions!
Salil
>
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > > >
> > > > > > + if (!buddy->bits[i])
> > > > > > + goto err_out_free;
> > > > > > + }
> > > > > > }
--
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 1/3] IB/mad: Fix an array index check
From: Hal Rosenstock @ 2016-11-21 20:17 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty
In-Reply-To: <396b3fe2-6a84-efed-07de-3e6381009ad1-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 11/21/2016 1:21 PM, Bart Van Assche wrote:
> The array ib_mad_mgmt_class_table.method_table has MAX_MGMT_CLASS
> (80) elements. Hence compare the array index with that value instead
> of with IB_MGMT_MAX_METHODS (128). This patch avoids that Coverity
> reports the following:
>
> Overrunning array class->method_table of 80 8-byte elements at element index 127 (byte offset 1016) using index convert_mgmt_class(mad_hdr->mgmt_class) (which evaluates to 127).
>
> Fixes: commit b7ab0b19a85f ("IB/mad: Verify mgmt class in received MADs")
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> ---
> drivers/infiniband/core/mad.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 40cbd6b..2395fe2 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -1746,7 +1746,7 @@ find_mad_agent(struct ib_mad_port_private *port_priv,
> if (!class)
> goto out;
> if (convert_mgmt_class(mad_hdr->mgmt_class) >=
> - IB_MGMT_MAX_METHODS)
> + ARRAY_SIZE(class->method_table))
> goto out;
> method = class->method_table[convert_mgmt_class(
> mad_hdr->mgmt_class)];
>
I think there is also similar thing in missing check in ib_register_mad_agent where:
/*
* Make sure MAD registration (if supplied)
* is non overlapping with any existing ones
*/
if (mad_reg_req) {
mgmt_class = convert_mgmt_class(mad_reg_req->mgmt_class);
if (!is_vendor_class(mgmt_class)) {
class = port_priv->version[mad_reg_req->
mgmt_class_version].class;
if (class) {
method = class->method_table[mgmt_class];
so here the class' method_table is also accessed without checking mgmt_class for range violation, right ?
-- Hal
--
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 3/3] IB/multicast: Check ib_find_pkey() return value
From: Bart Van Assche @ 2016-11-21 18:22 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty
In-Reply-To: <94eb11fc-b558-b994-c933-da784caefc53-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
This patch avoids that Coverity complains about not checking the
ib_find_pkey() return value.
Fixes: commit 547af76521b3 ("IB/multicast: Report errors on multicast groups if P_key changes")
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
drivers/infiniband/core/multicast.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/multicast.c b/drivers/infiniband/core/multicast.c
index e51b739..322cb67 100644
--- a/drivers/infiniband/core/multicast.c
+++ b/drivers/infiniband/core/multicast.c
@@ -518,8 +518,11 @@ static void join_handler(int status, struct ib_sa_mcmember_rec *rec,
process_join_error(group, status);
else {
int mgids_changed, is_mgid0;
- ib_find_pkey(group->port->dev->device, group->port->port_num,
- be16_to_cpu(rec->pkey), &pkey_index);
+
+ if (ib_find_pkey(group->port->dev->device,
+ group->port->port_num, be16_to_cpu(rec->pkey),
+ &pkey_index))
+ pkey_index = MCAST_INVALID_PKEY_INDEX;
spin_lock_irq(&group->port->lock);
if (group->state == MCAST_BUSY &&
--
2.10.2
--
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/3] IPoIB: Avoid reading an uninitialized member variable
From: Bart Van Assche @ 2016-11-21 18:21 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Erez Shitrit
In-Reply-To: <94eb11fc-b558-b994-c933-da784caefc53-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
This patch avoids that Coverity reports the following:
Using uninitialized value port_attr.state when calling printk
Fixes: commit 94232d9ce817 ("IPoIB: Start multicast join process only on active ports")
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Erez Shitrit <erezsh-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index 1909dd2..fddff40 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -575,8 +575,11 @@ void ipoib_mcast_join_task(struct work_struct *work)
if (!test_bit(IPOIB_FLAG_OPER_UP, &priv->flags))
return;
- if (ib_query_port(priv->ca, priv->port, &port_attr) ||
- port_attr.state != IB_PORT_ACTIVE) {
+ if (ib_query_port(priv->ca, priv->port, &port_attr)) {
+ ipoib_dbg(priv, "ib_query_port() failed\n");
+ return;
+ }
+ if (port_attr.state != IB_PORT_ACTIVE) {
ipoib_dbg(priv, "port state is not ACTIVE (state = %d) suspending join task\n",
port_attr.state);
return;
--
2.10.2
--
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 1/3] IB/mad: Fix an array index check
From: Bart Van Assche @ 2016-11-21 18:21 UTC (permalink / raw)
To: Doug Ledford
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sean Hefty
In-Reply-To: <94eb11fc-b558-b994-c933-da784caefc53-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
The array ib_mad_mgmt_class_table.method_table has MAX_MGMT_CLASS
(80) elements. Hence compare the array index with that value instead
of with IB_MGMT_MAX_METHODS (128). This patch avoids that Coverity
reports the following:
Overrunning array class->method_table of 80 8-byte elements at element index 127 (byte offset 1016) using index convert_mgmt_class(mad_hdr->mgmt_class) (which evaluates to 127).
Fixes: commit b7ab0b19a85f ("IB/mad: Verify mgmt class in received MADs")
Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
Cc: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
drivers/infiniband/core/mad.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 40cbd6b..2395fe2 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -1746,7 +1746,7 @@ find_mad_agent(struct ib_mad_port_private *port_priv,
if (!class)
goto out;
if (convert_mgmt_class(mad_hdr->mgmt_class) >=
- IB_MGMT_MAX_METHODS)
+ ARRAY_SIZE(class->method_table))
goto out;
method = class->method_table[convert_mgmt_class(
mad_hdr->mgmt_class)];
--
2.10.2
--
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 0/3] Address three Coverity complaints about RDMA code
From: Bart Van Assche @ 2016-11-21 18:20 UTC (permalink / raw)
To: Doug Ledford; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hello Doug,
The three patches in this series address warnings reported by Coverity
on the RDMA code. Please consider these patches for kernel version v4.10.
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 v5 2/9] IB/core: Replace semaphore sm_sem with an atomic wait
From: Linus Torvalds @ 2016-11-21 17:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Binoy Jayan, linux-rdma@vger.kernel.org, Mustafa Ismail, Lijun Ou,
Nicholas Bellinger, Leon Romanovsky, target-devel,
Tatyana E Nikolova, Doug Ledford, Jenny Derzhavetz, Sagi Grimberg,
Sean Hefty, Faisal Latif, Hal Rosenstock,
Linux Kernel Mailing List, Wei Hu(Xavier), Mark Brown, Mark Bloch,
Steve
In-Reply-To: <4374986.OFD8bCgUa1@wuerfel>
On Mon, Nov 21, 2016 at 8:52 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> Version of the series had replaced the semaphore with a completion
> here, which worked correctly, but one reviewer suggested using
> the wait_event() instead since it's confusing to have a completion
> starting out in 'completed' state.
Quite frankly, in that case just keep it as a semaphore.
So we have a few cases:
- completions are *slow*.
They are designed to be entirely synchronous, exactly so that you
can wait for a completion to finish, and then immediately free the
memory that the completion is in.
- completions used fro mutual exclusion are *confusing*.
Yes, they end up working as a counting semaphore, and yes, that's
by design, but at the same time, they are not _meant_ to be used as a
semaphore. The name matters. They are meant to be used the way they
are named: to be a "I'm done now" event. Don't use them for anything
else, because you *will* be confusing everybody else.
- open-coding wait-queues are really fragile and are *not* meant for exclusion.
They don't have lockdep checking, but more importantly, people
always invariably get the memory ordering wrong. And by "wrong" I mean
both "doesn't work" (because of insufficient memory ordering) and
"slow" (due to excessive memory ordering).
- mutexes are good for exclusion.
mutexes are both high-performance and non-confusing. Yes, they get
lockdep warnings, but that's usually a *good* thing. If you get
lockdep warnings from mutex use, you're almost certainly doing
something iffy.
mutexes do have a subtle downside: exactly because they are fast,
they are not entirely synchronous. You can't use them for completion
style notifications if you are going to release the memory they are
allocated in.
So mutexes need to be either statically allocated, or in
reference-counted allocations. That's so that the lifetime of the
memory is guaranteed to be cover all the users (including the wakers).
- semaphores are "old-fashioned mutexes". A mutex is better than a
semaphore, but a semaphore is better than just about all the other
alternatives. There's nothing _wrong_ with using a semaphore per se.
In this case, either use a semaphore or a mutex. If you are doing
mutual exclusion, those are really the only two acceptable sleeping
models.
Linus
^ permalink raw reply
* Re: [PATCH v2] infiniband: remove WARN that is not kernel bug
From: Leon Romanovsky @ 2016-11-21 17:38 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Dmitry Vyukov, syzkaller, Valdis.Kletnieks-PjAqaU27lzQ,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w, Hal Rosenstock,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, LKML
In-Reply-To: <20161121165253.GA22237-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 523 bytes --]
On Mon, Nov 21, 2016 at 09:52:53AM -0700, Jason Gunthorpe wrote:
> On Mon, Nov 21, 2016 at 02:14:08PM +0200, Leon Romanovsky wrote:
> > >
> > > In ib_ucm_write function there is a wrong prefix:
> > >
> > > + pr_err_once("ucm_write: process %d (%s) tried to do something hinky\n",
> >
> > I did it intentionally to have the same errors for all flows.
>
> Lets actually use a good message too please?
>
> pr_err_once("ucm_write: process %d (%s) changed security contexts after opening FD, this is not allowed.\n",
>
> Jason
[-- Attachment #1.2: 0001-IB-core-qib-Remove-WARN-that-is-not-kernel-bug.patch --]
[-- Type: text/x-diff, Size: 3630 bytes --]
From 70f95b2d35aea42e5b97e7d27ab2f4e8effcbe67 Mon Sep 17 00:00:00 2001
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Date: Mon, 21 Nov 2016 13:30:59 +0200
Subject: [PATCH rdma-next V2] IB/{core, qib}: Remove WARN that is not kernel bug
WARNINGs mean kernel bugs, in this case, they are placed
to mark programming errors and/or malicious attempts.
BUG/WARNs that are not kernel bugs hinder automated testing efforts.
Signed-off-by: Dmitry Vyukov <dvyukov-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
* Improved error prints.
---
drivers/infiniband/core/ucm.c | 5 ++++-
drivers/infiniband/core/ucma.c | 5 ++++-
drivers/infiniband/core/uverbs_main.c | 5 ++++-
drivers/infiniband/hw/qib/qib_file_ops.c | 5 ++++-
4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index 7713ef0..579f9a7 100644
--- a/drivers/infiniband/core/ucm.c
+++ b/drivers/infiniband/core/ucm.c
@@ -1104,8 +1104,11 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
struct ib_ucm_cmd_hdr hdr;
ssize_t result;
- if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+ if (!ib_safe_file_access(filp)) {
+ pr_err_once("ucm_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
+ task_tgid_vnr(current), current->comm);
return -EACCES;
+ }
if (len < sizeof(hdr))
return -EINVAL;
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 9520154..e12f8fa 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1584,8 +1584,11 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
struct rdma_ucm_cmd_hdr hdr;
ssize_t ret;
- if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+ if (!ib_safe_file_access(filp)) {
+ pr_err_once("ucma_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
+ task_tgid_vnr(current), current->comm);
return -EACCES;
+ }
if (len < sizeof(hdr))
return -EINVAL;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 44b1104..38c79ad 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -746,8 +746,11 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
int srcu_key;
ssize_t ret;
- if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+ if (!ib_safe_file_access(filp)) {
+ pr_err_once("uverbs_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
+ task_tgid_vnr(current), current->comm);
return -EACCES;
+ }
if (count < sizeof hdr)
return -EINVAL;
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index 382466a..2d1eacf 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -2066,8 +2066,11 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
ssize_t ret = 0;
void *dest;
- if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
+ if (!ib_safe_file_access(fp)) {
+ pr_err_once("qib_write: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
+ task_tgid_vnr(current), current->comm);
return -EACCES;
+ }
if (count < sizeof(cmd.type)) {
ret = -EINVAL;
--
2.7.4
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related
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