LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: jackm <jackm@dev.mellanox.co.il>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: "Håkon Bugge" <haakon.bugge@oracle.com>,
	"Hans Westgaard Ry" <hans.westgaard.ry@oracle.com>,
	"Doug Ledford" <dledford@redhat.com>,
	"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, "Leon Romanovsky" <leon@kernel.org>
Subject: Re: [PATCH] IB/mad: Use ID allocator routines to allocate agent number
Date: Wed, 30 May 2018 11:02:16 +0300	[thread overview]
Message-ID: <20180530110216.00000913@dev.mellanox.co.il> (raw)
In-Reply-To: <20180529164032.GB18457@ziepe.ca>

On Tue, 29 May 2018 10:40:32 -0600
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.
> 
> 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

  parent reply	other threads:[~2018-05-30  8:02 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
2018-05-30  8:02       ` jackm [this message]
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=20180530110216.00000913@dev.mellanox.co.il \
    --to=jackm@dev.mellanox.co.il \
    --cc=danielj@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=haakon.bugge@oracle.com \
    --cc=hans.westgaard.ry@oracle.com \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --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).