LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: "Håkon Bugge" <haakon.bugge@oracle.com>
Cc: Hans Westgaard Ry <hans.westgaard.ry@oracle.com>,
	Doug Ledford <dledford@redhat.com>,
	Jack Morgenstein <jackm@dev.mellanox.co.il>,
	Daniel Jurgens <danielj@mellanox.com>,
	Parav Pandit <parav@mellanox.com>,
	Pravin Shedge <pravin.shedge4linux@gmail.com>,
	OFED mailing list <linux-rdma@vger.kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent number
Date: Wed, 30 May 2018 09:15:47 -0600	[thread overview]
Message-ID: <20180530151547.GC30754@ziepe.ca> (raw)
In-Reply-To: <0E56560E-F577-43A6-9F18-B7AC7B5DB213@oracle.com>

On Wed, May 30, 2018 at 09:32:02AM +0200, Håkon Bugge wrote:
> 
> 
> > On 29 May 2018, at 18:40, Jason Gunthorpe <jgg@ziepe.ca> 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@ziepe.ca> 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.
> 
> Initially, I thought that too, but consulted the spec which states:
> 
> C13-18.1.1: 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. []
> 
> "currently executing operation" that is.

'currently executing operation' encompasses a wide range of behaviors
and it is not just 'completing the MAD at the local node'

You have to exceed all the timeouts before a single node can be
certain that the condition is satisfied.

This is why we just simply maximize the time between allocations, and
encourage the user providing the rest of the tid to do the same.

> > Preventing re-use seems like a seperate issue from limiting the range
> > to be compatible with mlx4.
> 
> Yes. But a v2 of Hans' patch using idr_alloc_cyclic() solves both issues.

That might work, but still have to get rid of the sysctl

Jason

  reply	other threads:[~2018-05-30 15:15 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29  7:38 Hans Westgaard Ry
2018-05-29  8:54 ` Leon Romanovsky
2018-05-29  9:54   ` Leon Romanovsky
2018-05-30  8:18     ` jackm
2018-05-29 15:49 ` Jason Gunthorpe
2018-05-29 16:16   ` Håkon Bugge
2018-05-29 16:40     ` Jason Gunthorpe
2018-05-30  7:32       ` Håkon Bugge
2018-05-30 15:15         ` Jason Gunthorpe [this message]
2018-05-30  8:02       ` jackm
2018-05-30 12:22         ` Hans Westgaard Ry
2018-05-30 15:10           ` Jason Gunthorpe
2018-05-30 20:07             ` Håkon Bugge
2018-05-30 22:09               ` Jason Gunthorpe
2018-05-31 19:54                 ` Håkon Bugge
2018-06-07 10:52 ` [PATCH v2 0/2] IB:mad " Hans Westgaard Ry
2018-06-07 10:52   ` [PATCH v2 1/2] idr: Add ida_simple_get_cyclic Hans Westgaard Ry
2018-06-07 10:52   ` [PATCH v2 2/2] IB/mad: Use ID allocator routines to allocate agent number Hans Westgaard Ry
2018-06-07 11:14 ` [PATCH v3 0/2] IB:mad " Hans Westgaard Ry
2018-06-07 11:14   ` [PATCH v3 1/2] idr: Add ida_simple_get_cyclic Hans Westgaard Ry
2018-06-07 18:50     ` Jason Gunthorpe
2018-06-07 11:14   ` [PATCH v3 2/2] IB/mad: Use ID allocator routines to allocate agent number Hans Westgaard Ry
2018-06-07 15:37     ` Matthew Wilcox
2018-06-07 17:59       ` Håkon Bugge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180530151547.GC30754@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=danielj@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=haakon.bugge@oracle.com \
    --cc=hans.westgaard.ry@oracle.com \
    --cc=jackm@dev.mellanox.co.il \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=pravin.shedge4linux@gmail.com \
    --subject='Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent number' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).