On 05/30/2018 10:02 AM, jackm wrote:
On Tue, 29 May 2018 10:40:32 -0600
Jason Gunthorpe <jgg@xxxxxxxx> wrote:
On Tue, May 29, 2018 at 06:16:14PM +0200, Håkon Bugge wrote:
On 29 May 2018, at 17:49, Jason Gunthorpe <jgg@xxxxxxxx> wrote:
On Tue, May 29, 2018 at 09:38:08AM +0200, Hans Westgaard Ry
wrote:
The agent TID is a 64 bit value split in two dwords. The least
significant dword is the TID running counter. The most
significant dword is the agent number. In the CX-3 shared port
model, the mlx4 driver uses the most significant byte of the
agent number to store the slave number, making agent numbers
greater and equal to 2^24 (3 bytes) unusable.
There is no reason for this to be an ida, just do something like
mad_agent_priv->agent.hi_tid =
atomic_inc_return(&ib_mad_client_id) &
mad_agent_priv->ib_dev->tid_mask;
And have the driver set tid_mask to 3 bytes of 0xFF
The issue is that some of the mad agents are long-lived, so you will
wrap and use the same TID twice.
We already have that problem, and using ida is problematic because we
need to maximize the time between TID re-use, which ida isn't doing.
Preventing re-use seems like a seperate issue from limiting the range
to be compatible with mlx4.
Preventing immediate re-use can be accomplished by judicious use of the
start argument (second argument) in the call to ida_simple_get (to
introduce hysteresis into the id allocations).
For example, can do something like:
static atomic_t ib_mad_client_id_min = ATOMIC_INIT(1);
...
ib_mad_client_id = ida_simple_get(&ib_mad_client_ids,
atomic_read(&ib_mad_client_id_min),
ib_mad_sysctl_client_id_max,
GFP_KERNEL);
....
if (!(ib_mad_client_id % 1000) ||
ib_mad_sysctl_client_id_max - ib_mad_client_id <= 1000)
atomic_set(&ib_mad_client_id_min, 1);
else
atomic_set(&ib_mad_client_id_min, ib_mad_client_id + 1);
The above avoids immediate re-use of ids, and only searches for past
freed ids if the last allocated-id is zero mod 1000.
This is just suggestion -- will probably need some variation of the
above to handle what happens over time (i.e., to not depend on the
modulo operation to reset the search start to 1), to properly handle
how we deal with the start value when we are close to the allowed
client_id_max, and also to implement some sort of locking.
-Jack
We came up with this code snippet which we think handles both preventing
immediate re-use and too big/wrapping...
max = mad_agent_priv->ib_dev->tid_max;
start = atomic_inc_return(&start);
retry:
tid = ida_simple_get(&ib_mad_client_ids, start, max, GFP_KERNEL);
if (unlikely(tid == -ENOSPC)) {
spin_lock_irq_save();
tid = ida_simple_get(&ib_mad_client_ids, start, max,
GFP_ATOMIC);
if (unlikely(tid == -ENOSPC)) {
atomic_set(&start, 1);
spin_unlock_irq_restore();
goto retry;
}
spin_unlock_irq_restore();
}
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
