Two kernel threads may get the same value for agent.hi_tid, if the
agents are registered for different ports. As of now, this works, as
the agent list is per port.
It is however confusing and not future robust. Hence, making it
atomic.
Signed-off-by: Håkon Bugge <[email protected]>
Reviewed-by: Jack Morgenstein <[email protected]>
---
drivers/infiniband/core/mad.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index c50596f7f98a..b28452a55a08 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -59,7 +59,7 @@ module_param_named(recv_queue_size, mad_recvq_size, int, 0444);
MODULE_PARM_DESC(recv_queue_size, "Size of receive queue in number of work requests");
static struct list_head ib_mad_port_list;
-static u32 ib_mad_client_id = 0;
+static atomic_t ib_mad_client_id = ATOMIC_INIT(0);
/* Port list lock */
static DEFINE_SPINLOCK(ib_mad_port_list_lock);
@@ -377,7 +377,7 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
}
spin_lock_irqsave(&port_priv->reg_lock, flags);
- mad_agent_priv->agent.hi_tid = ++ib_mad_client_id;
+ mad_agent_priv->agent.hi_tid = atomic_inc_return(&ib_mad_client_id);
/*
* Make sure MAD registration (if supplied)
--
2.13.6
>
> Two kernel threads may get the same value for agent.hi_tid, if the agents are
> registered for different ports. As of now, this works, as the agent list is per port.
>
> It is however confusing and not future robust. Hence, making it atomic.
>
> Signed-off-by: Håkon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
> ---
> drivers/infiniband/core/mad.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c index
> c50596f7f98a..b28452a55a08 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -59,7 +59,7 @@ module_param_named(recv_queue_size, mad_recvq_size,
> int, 0444); MODULE_PARM_DESC(recv_queue_size, "Size of receive queue in
> number of work requests");
>
> static struct list_head ib_mad_port_list;
> -static u32 ib_mad_client_id = 0;
> +static atomic_t ib_mad_client_id = ATOMIC_INIT(0);
>
> /* Port list lock */
> static DEFINE_SPINLOCK(ib_mad_port_list_lock);
> @@ -377,7 +377,7 @@ struct ib_mad_agent *ib_register_mad_agent(struct
> ib_device *device,
> }
>
> spin_lock_irqsave(&port_priv->reg_lock, flags);
> - mad_agent_priv->agent.hi_tid = ++ib_mad_client_id;
> + mad_agent_priv->agent.hi_tid =
> atomic_inc_return(&ib_mad_client_id);
>
> /*
> * Make sure MAD registration (if supplied)
> --
> 2.13.6
On 2018/4/18 22:24, Håkon Bugge wrote:
> Two kernel threads may get the same value for agent.hi_tid, if the
> agents are registered for different ports. As of now, this works, as
> the agent list is per port.
>
> It is however confusing and not future robust. Hence, making it
> atomic.
>
> Signed-off-by: Håkon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
Reviewed-by: Zhu Yanjun <[email protected]>
> ---
> drivers/infiniband/core/mad.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index c50596f7f98a..b28452a55a08 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -59,7 +59,7 @@ module_param_named(recv_queue_size, mad_recvq_size, int, 0444);
> MODULE_PARM_DESC(recv_queue_size, "Size of receive queue in number of work requests");
>
> static struct list_head ib_mad_port_list;
> -static u32 ib_mad_client_id = 0;
> +static atomic_t ib_mad_client_id = ATOMIC_INIT(0);
>
> /* Port list lock */
> static DEFINE_SPINLOCK(ib_mad_port_list_lock);
> @@ -377,7 +377,7 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
> }
>
> spin_lock_irqsave(&port_priv->reg_lock, flags);
> - mad_agent_priv->agent.hi_tid = ++ib_mad_client_id;
> + mad_agent_priv->agent.hi_tid = atomic_inc_return(&ib_mad_client_id);
>
> /*
> * Make sure MAD registration (if supplied)
On Wed, 2018-04-18 at 16:24 +0200, Håkon Bugge wrote:
> Two kernel threads may get the same value for agent.hi_tid, if the
> agents are registered for different ports. As of now, this works, as
> the agent list is per port.
>
> It is however confusing and not future robust. Hence, making it
> atomic.
>
People sometimes underestimate the performance penalty of atomic ops.
Every atomic op is the equivalent of a spin_lock/spin_unlock pair. This
is why two atomics are worse than taking a spin_lock, doing what you
have to do, and releasing the spin_lock. Is this really what you want
for a "confusing, let's make it robust" issue?
--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
On Thu, Apr 19, 2018 at 11:55:55PM -0400, Doug Ledford wrote:
> On Wed, 2018-04-18 at 16:24 +0200, Håkon Bugge wrote:
> > Two kernel threads may get the same value for agent.hi_tid, if the
> > agents are registered for different ports. As of now, this works, as
> > the agent list is per port.
> >
> > It is however confusing and not future robust. Hence, making it
> > atomic.
> >
>
> People sometimes underestimate the performance penalty of atomic ops.
> Every atomic op is the equivalent of a spin_lock/spin_unlock pair. This
> is why two atomics are worse than taking a spin_lock, doing what you
> have to do, and releasing the spin_lock. Is this really what you want
> for a "confusing, let's make it robust" issue?
But it is on the ib_register_mad_agent() path which is not a
performance path..
This actually looks like a genuine bug, why is it described only as
'confusing'? ib_register_mad_agent is callable from userspace, so at
least two userspace agents can race and get the same TID's.
TIDs need to be globally unique on the entire machine.
Jason
+Jack
> On 20 Apr 2018, at 17:34, Jason Gunthorpe <[email protected]> wrote:
>
> On Thu, Apr 19, 2018 at 11:55:55PM -0400, Doug Ledford wrote:
>> On Wed, 2018-04-18 at 16:24 +0200, Håkon Bugge wrote:
>>> Two kernel threads may get the same value for agent.hi_tid, if the
>>> agents are registered for different ports. As of now, this works, as
>>> the agent list is per port.
>>>
>>> It is however confusing and not future robust. Hence, making it
>>> atomic.
>>>
>>
>> People sometimes underestimate the performance penalty of atomic ops.
>> Every atomic op is the equivalent of a spin_lock/spin_unlock pair.
Well, may be this holds true if the mutex and the variable is located in the same cacheline.
>> This
>> is why two atomics are worse than taking a spin_lock, doing what you
>> have to do, and releasing the spin_lock. Is this really what you want
>> for a "confusing, let's make it robust" issue?
>
> But it is on the ib_register_mad_agent() path which is not a
> performance path..
>
> This actually looks like a genuine bug, why is it described only as
> 'confusing'? ib_register_mad_agent is callable from userspace, so at
> least two userspace agents can race and get the same TID’s.
My understanding is that every lookup is using the {port, TID} tuple. As such, it is not a bug, but, very confusing.
> TIDs need to be globally unique on the entire machine.
If you are correct Jason, let me reword the commit message.
Thxs, Håkon
On Mon, 23 Apr 2018 16:19:57 +0200
Håkon Bugge <[email protected]> wrote:
> >
> > This actually looks like a genuine bug, why is it described only as
> > 'confusing'? ib_register_mad_agent is callable from userspace, so at
> > least two userspace agents can race and get the same TID’s.
>
> My understanding is that every lookup is using the {port, TID} tuple.
> As such, it is not a bug, but, very confusing.
Haakon, you are correct (see snippet from the IB spec, below).
We will NOT have a situation where there are 2 threads/apps
with the same agent ID on the *same port* (accessing the agent ID
allocator is protected by a per-port spinlock). Having the same agent ID
on DIFFERENT ports is OK.
Thus, there is NO bug here. (But as Haakon says, IMHO it is more robust
to avoid having the same agent ID for 2 agents even if those agents are
on different ports).
>
> > TIDs need to be globally unique on the entire machine.
>
Jason, that is not exactly correct.
From the IB Spec 1.3, C13-18.1.1 (in section 13.4.6.4 - TransactionID
usage):
"When initiating a new operation, MADHeader:TransactionID
shall be set to such a value that within that MAD the combination of
TID, SGID, and MgmtClass is different from that of any other currently
executing operation. If the MAD does not have a GRH, its SLID is used
in the combination in place of an SGID."
Since the SGID/SLID is different for each port, the per-port guarantee
of no 2 agents receiving the same agent-ID value is sufficient.
-Jack
> On 23 Apr 2018, at 21:16, jackm <[email protected]> wrote:
>
> On Mon, 23 Apr 2018 16:19:57 +0200
> Håkon Bugge <[email protected]> wrote:
>
>
>>>
>>> This actually looks like a genuine bug, why is it described only as
>>> 'confusing'? ib_register_mad_agent is callable from userspace, so at
>>> least two userspace agents can race and get the same TID’s.
>>
>> My understanding is that every lookup is using the {port, TID} tuple.
>> As such, it is not a bug, but, very confusing.
> Haakon, you are correct (see snippet from the IB spec, below).
>
> We will NOT have a situation where there are 2 threads/apps
> with the same agent ID on the *same port* (accessing the agent ID
> allocator is protected by a per-port spinlock). Having the same agent ID
> on DIFFERENT ports is OK.
> Thus, there is NO bug here. (But as Haakon says, IMHO it is more robust
> to avoid having the same agent ID for 2 agents even if those agents are
> on different ports).
>
>>
>>> TIDs need to be globally unique on the entire machine.
>>
> Jason, that is not exactly correct.
>
> From the IB Spec 1.3, C13-18.1.1 (in section 13.4.6.4 - TransactionID
> usage):
> "When initiating a new operation, MADHeader:TransactionID
> shall be set to such a value that within that MAD the combination of
> TID, SGID, and MgmtClass is different from that of any other currently
> executing operation. If the MAD does not have a GRH, its SLID is used
> in the combination in place of an SGID."
>
> Since the SGID/SLID is different for each port, the per-port guarantee
> of no 2 agents receiving the same agent-ID value is sufficient.
>
> -Jack
Shall I interpret this silence as my commit is good to go or that I should add Jack’s tangible information to the commit message?
Thxs, Håkon
On Thu, 26 Apr 2018 18:06:10 +0200
Håkon Bugge <[email protected]> wrote:
> > On 23 Apr 2018, at 21:16, jackm <[email protected]> wrote:
> >
> > On Mon, 23 Apr 2018 16:19:57 +0200
> > Håkon Bugge <[email protected]> wrote:
> >
> >
> >>>
> >>> This actually looks like a genuine bug, why is it described only
> >>> as 'confusing'? ib_register_mad_agent is callable from userspace,
> >>> so at least two userspace agents can race and get the same
> >>> TID’s.
> >>
> >> My understanding is that every lookup is using the {port, TID}
> >> tuple. As such, it is not a bug, but, very confusing.
> > Haakon, you are correct (see snippet from the IB spec, below).
> >
> > We will NOT have a situation where there are 2 threads/apps
> > with the same agent ID on the *same port* (accessing the agent ID
> > allocator is protected by a per-port spinlock). Having the same
> > agent ID on DIFFERENT ports is OK.
> > Thus, there is NO bug here. (But as Haakon says, IMHO it is more
> > robust to avoid having the same agent ID for 2 agents even if those
> > agents are on different ports).
> >
> >>
> >>> TIDs need to be globally unique on the entire machine.
> >>
> > Jason, that is not exactly correct.
> >
> > From the IB Spec 1.3, C13-18.1.1 (in section 13.4.6.4 -
> > TransactionID usage):
> > "When initiating a new operation, MADHeader:TransactionID
> > shall be set to such a value that within that MAD the combination of
> > TID, SGID, and MgmtClass is different from that of any other
> > currently executing operation. If the MAD does not have a GRH, its
> > SLID is used in the combination in place of an SGID."
> >
> > Since the SGID/SLID is different for each port, the per-port
> > guarantee of no 2 agents receiving the same agent-ID value is
> > sufficient.
> >
> > -Jack
>
> Shall I interpret this silence as my commit is good to go or that I
> should add Jack’s tangible information to the commit message?
>
>
Haakon, I think Jason is on vacation this week. Best to wait for his
response.
-Jack
> Thxs, Håkon
>
On Thu, 2018-04-26 at 20:51 +0200, Håkon Bugge wrote:
> > Jason is out this week. I'll end up processing this one (probably later
> > today). But I’ll fix up the commit message to suit my tastes when I do.
>
> Thank you, Doug and Jack,
I reworded the commit message, let me know if you think I worded it
wrong:
commit 69f01b81539c62f3dd96f9f02138ad7b839a0c70 (HEAD -> k.o/wip/dl-for-rc)
Author: Håkon Bugge <[email protected]>
Date: Wed Apr 18 16:24:50 2018 +0200
IB/core: Make ib_mad_client_id atomic
Currently, kernel protects access to the agent ID allocator on a per
port basis using a spinlock, so it is impossible for two apps/threads on
the same port to get the same TID, but it is entirely possible for two
threads on different ports to end up with the same TID. As this can be
confusing (regardless of it being legal according to the IB Spec 1.3,
C13-18.1.1, in section 13.4.6.4 - TransactionID usage: "Then initiating
a new operation, MADHeader:TransactionID shall be set to such a value
that within that MAD the combination of TIG, SGID, and MgmtClass is
different from that of any other currently executing operation. If the
MAD does not have a GRH, its SLID is used in the combination in place of
an SGID." which guarantees we are legal because our different ports will
have different SGID/SLID creating a unique tuple even if the TIDs are
identical), and as we might want to open the TID allocator up to more
parallel usage later, make the TID an atomic type so that no two
allocations, regardless of port number, will be the same.
Signed-off-by: Håkon Bugge <[email protected]>
Reviewed-by: Jack Morgenstein <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Zhu Yanjun <[email protected]>
Signed-off-by: Doug Ledford <[email protected]>
--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
> On 27 Apr 2018, at 21:08, Doug Ledford <[email protected]> wrote:
>
> On Thu, 2018-04-26 at 20:51 +0200, Håkon Bugge wrote:
>>> Jason is out this week. I'll end up processing this one (probably later
>>> today). But I’ll fix up the commit message to suit my tastes when I do.
>>
>> Thank you, Doug and Jack,
>
> I reworded the commit message, let me know if you think I worded it
> wrong:
>
> commit 69f01b81539c62f3dd96f9f02138ad7b839a0c70 (HEAD -> k.o/wip/dl-for-rc)
> Author: Håkon Bugge <[email protected]>
> Date: Wed Apr 18 16:24:50 2018 +0200
>
> IB/core: Make ib_mad_client_id atomic
>
> Currently, kernel protects access to the agent ID allocator on a per
> port basis using a spinlock, so it is impossible for two apps/threads on
> the same port to get the same TID, but it is entirely possible for two
> threads on different ports to end up with the same TID. As this can be
> confusing (regardless of it being legal according to the IB Spec 1.3,
> C13-18.1.1, in section 13.4.6.4 - TransactionID usage: "Then initiating
> a new operation, MADHeader:TransactionID shall be set to such a value
> that within that MAD the combination of TIG, SGID, and MgmtClass is
> different from that of any other currently executing operation. If the
> MAD does not have a GRH, its SLID is used in the combination in place of
> an SGID." which guarantees we are legal because our different ports will
> have different SGID/SLID creating a unique tuple even if the TIDs are
> identical), and as we might want to open the TID allocator up to more
> parallel usage later, make the TID an atomic type so that no two
> allocations, regardless of port number, will be the same.
The wording is OK per se, but the last sentence spans 12 lines - and as such - a little bit hard to comprehend. I suggest just to refer to IB Spec 1.3, clause C13-18.1.1 and not quote it and make the last sub-sentence (starting with “and as we might want to open”) a sentence by its own.
Thxs, Håkon
>
> Signed-off-by: Håkon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Reviewed-by: Zhu Yanjun <[email protected]>
> Signed-off-by: Doug Ledford <[email protected]>
>
> --
> Doug Ledford <[email protected]>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
> > > TIDs need to be globally unique on the entire machine.
> Jason, that is not exactly correct.
The expecation for /dev/umad users is that they all receive locally
unique TID prefixes. The kernel may be OK to keep things port-specific
but it is slightly breaking the API we are presenting to userspace to
allow them to alias..
Jason
On Mon, 2018-04-30 at 08:49 -0600, Jason Gunthorpe wrote:
> On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
>
> > > > TIDs need to be globally unique on the entire machine.
> > Jason, that is not exactly correct.
>
> The expecation for /dev/umad users is that they all receive locally
> unique TID prefixes. The kernel may be OK to keep things port-specific
> but it is slightly breaking the API we are presenting to userspace to
> allow them to alias..
>
> Jason
Would people be happier with this commit message then:
IB/core: Make ib_mad_client_id atomic
Currently, the kernel protects access to the agent ID allocator on a per
port basis using a spinlock, so it is impossible for two apps/threads on
the same port to get the same TID, but it is entirely possible for two
threads on different ports to end up with the same TID.
As this can be confusing (regardless of it being legal according to the
IB Spec 1.3, C13-18.1.1, in section 13.4.6.4 - TransactionID usage),
and as the rdma-core user space API for /dev/umad devices implies unique
TIDs even across ports, make the TID an atomic type so that no two
allocations, regardless of port number, will be the same.
Signed-off-by: Håkon Bugge <[email protected]>
Reviewed-by: Jack Morgenstein <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Reviewed-by: Zhu Yanjun <[email protected]>
Signed-off-by: Doug Ledford <[email protected]>
--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
Looks fine to me.
> -----Original Message-----
> From: Doug Ledford [mailto:[email protected]]
> Sent: Monday, April 30, 2018 10:11 AM
> To: Jason Gunthorpe <[email protected]>; jackm <[email protected]>
> Cc: Håkon Bugge <[email protected]>; Hiatt, Don
> <[email protected]>; Dasaratharaman Chandramouli
> <[email protected]>; Weiny, Ira <[email protected]>;
> Hefty, Sean <[email protected]>; OFED mailing list <linux-
> [email protected]>; [email protected]
> Subject: Re: [PATCH] IB/core: Make ib_mad_client_id atomic
>
> On Mon, 2018-04-30 at 08:49 -0600, Jason Gunthorpe wrote:
> > On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
> >
> > > > > TIDs need to be globally unique on the entire machine.
> > > Jason, that is not exactly correct.
> >
> > The expecation for /dev/umad users is that they all receive locally
> > unique TID prefixes. The kernel may be OK to keep things port-specific
> > but it is slightly breaking the API we are presenting to userspace to
> > allow them to alias..
> >
> > Jason
>
> Would people be happier with this commit message then:
>
> IB/core: Make ib_mad_client_id atomic
>
> Currently, the kernel protects access to the agent ID allocator on a per port basis
> using a spinlock, so it is impossible for two apps/threads on the same port to get
> the same TID, but it is entirely possible for two threads on different ports to end
> up with the same TID.
>
> As this can be confusing (regardless of it being legal according to the IB Spec 1.3,
> C13-18.1.1, in section 13.4.6.4 - TransactionID usage), and as the rdma-core user
> space API for /dev/umad devices implies unique TIDs even across ports, make
> the TID an atomic type so that no two allocations, regardless of port number, will
> be the same.
>
> Signed-off-by: Håkon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Reviewed-by: Zhu Yanjun <[email protected]>
> Signed-off-by: Doug Ledford <[email protected]>
>
>
> --
> Doug Ledford <[email protected]>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
On Mon, Apr 30, 2018 at 01:10:49PM -0400, Doug Ledford wrote:
> On Mon, 2018-04-30 at 08:49 -0600, Jason Gunthorpe wrote:
> > On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
> >
> > > > > TIDs need to be globally unique on the entire machine.
> > > Jason, that is not exactly correct.
> >
> > The expecation for /dev/umad users is that they all receive locally
> > unique TID prefixes. The kernel may be OK to keep things port-specific
> > but it is slightly breaking the API we are presenting to userspace to
> > allow them to alias..
> >
> > Jason
>
> Would people be happier with this commit message then:
>
> IB/core: Make ib_mad_client_id atomic
>
> Currently, the kernel protects access to the agent ID allocator on a per
> port basis using a spinlock, so it is impossible for two apps/threads on
> the same port to get the same TID, but it is entirely possible for two
> threads on different ports to end up with the same TID.
>
> As this can be confusing (regardless of it being legal according to the
> IB Spec 1.3, C13-18.1.1, in section 13.4.6.4 - TransactionID usage),
> and as the rdma-core user space API for /dev/umad devices implies unique
> TIDs even across ports, make the TID an atomic type so that no two
> allocations, regardless of port number, will be the same.
>
> Signed-off-by: Håkon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Reviewed-by: Zhu Yanjun <[email protected]>
> Signed-off-by: Doug Ledford <[email protected]>
fine for me
Jason
On Mon, 30 Apr 2018 13:10:49 -0400
Doug Ledford <[email protected]> wrote:
Looks good!
-Jack
> On Mon, 2018-04-30 at 08:49 -0600, Jason Gunthorpe wrote:
> > On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
> >
> > > > > TIDs need to be globally unique on the entire machine.
> > > Jason, that is not exactly correct.
> >
> > The expecation for /dev/umad users is that they all receive locally
> > unique TID prefixes. The kernel may be OK to keep things
> > port-specific but it is slightly breaking the API we are presenting
> > to userspace to allow them to alias..
> >
> > Jason
>
> Would people be happier with this commit message then:
>
> IB/core: Make ib_mad_client_id atomic
>
> Currently, the kernel protects access to the agent ID allocator on a
> per port basis using a spinlock, so it is impossible for two
> apps/threads on the same port to get the same TID, but it is entirely
> possible for two threads on different ports to end up with the same
> TID.
>
> As this can be confusing (regardless of it being legal according to
> the IB Spec 1.3, C13-18.1.1, in section 13.4.6.4 - TransactionID
> usage), and as the rdma-core user space API for /dev/umad devices
> implies unique TIDs even across ports, make the TID an atomic type so
> that no two allocations, regardless of port number, will be the same.
>
> Signed-off-by: H?kon Bugge <[email protected]>
> Reviewed-by: Jack Morgenstein <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Reviewed-by: Zhu Yanjun <[email protected]>
> Signed-off-by: Doug Ledford <[email protected]>
>
>
> On 1 May 2018, at 06:38, jackm <[email protected]> wrote:
>
> On Mon, 30 Apr 2018 13:10:49 -0400
> Doug Ledford <[email protected]> wrote:
>
> Looks good!
Yes, absolutely!
Håkon
>
> -Jack
>
>> On Mon, 2018-04-30 at 08:49 -0600, Jason Gunthorpe wrote:
>>> On Mon, Apr 23, 2018 at 10:16:18PM +0300, jackm wrote:
>>>
>>>>>> TIDs need to be globally unique on the entire machine.
>>>> Jason, that is not exactly correct.
>>>
>>> The expecation for /dev/umad users is that they all receive locally
>>> unique TID prefixes. The kernel may be OK to keep things
>>> port-specific but it is slightly breaking the API we are presenting
>>> to userspace to allow them to alias..
>>>
>>> Jason
>>
>> Would people be happier with this commit message then:
>>
>> IB/core: Make ib_mad_client_id atomic
>>
>> Currently, the kernel protects access to the agent ID allocator on a
>> per port basis using a spinlock, so it is impossible for two
>> apps/threads on the same port to get the same TID, but it is entirely
>> possible for two threads on different ports to end up with the same
>> TID.
>>
>> As this can be confusing (regardless of it being legal according to
>> the IB Spec 1.3, C13-18.1.1, in section 13.4.6.4 - TransactionID
>> usage), and as the rdma-core user space API for /dev/umad devices
>> implies unique TIDs even across ports, make the TID an atomic type so
>> that no two allocations, regardless of port number, will be the same.
>>
>> Signed-off-by: Håkon Bugge <[email protected]>
>> Reviewed-by: Jack Morgenstein <[email protected]>
>> Reviewed-by: Ira Weiny <[email protected]>
>> Reviewed-by: Zhu Yanjun <[email protected]>
>> Signed-off-by: Doug Ledford <[email protected]>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html