On Fri, Oct 06, 2017 at 09:03:25AM +0200, Nélio Laranjeiro wrote:
> On Thu, Oct 05, 2017 at 09:59:58PM -0700, Yongseok Koh wrote:
> > On Thu, Oct 05, 2017 at 02:49:47PM +0200, Nelio Laranjeiro wrote:
> > [...]
> > > +struct mlx5_hrxq*
> > > +mlx5_priv_hrxq_get(struct priv *priv, uint8_t *rss_key, uint8_t
> > > rss_key_len,
> > > + uint64_t hash_fields, uint16_t queues[], uint16_t queues_n)
> > > +{
> > > + struct mlx5_hrxq *hrxq;
> > > +
> > > + LIST_FOREACH(hrxq, &priv->hrxqs, next) {
> > > + struct mlx5_ind_table_ibv *ind_tbl;
> > > +
> > > + if (hrxq->rss_key_len != rss_key_len)
> > > + continue;
> > > + if (memcmp(hrxq->rss_key, rss_key, rss_key_len))
> > > + continue;
> > > + if (hrxq->hash_fields != hash_fields)
> > > + continue;
> > > + ind_tbl = mlx5_priv_ind_table_ibv_get(priv, queues, queues_n);
> > > + if (!ind_tbl)
> > > + continue;
> > > + if (ind_tbl != hrxq->ind_table) {
> > > + mlx5_priv_ind_table_ibv_release(priv, ind_tbl);
> >
> > As one hrxq can have only one ind_tbl, it looks unnecessary to increment
> > refcnt
> > of ind_tbl. As long as a hrxq exist, its ind_tbl can't be destroyed. So,
> > it's
> > safe. How about moving up this _release() outside of this if-clause and
> > remove
> > _release() in _hrxq_release()?
>
> This is right, but in the other side, an indirection table can be used
> by several hash rx queues, that is the main reason why they have their
> own reference counter.
>
>
> +-------+ +-------+
> | Hrxq | | Hrxq |
> | r = 1 | | r = 1 |
> +-------+ +-------+
> | |
> v v
> +-------------------+
> | indirection table |
> | r = 2 |
> +-------------------+
>
> Seems logical to make the Indirection table counter evolve the same way
> as the hash rx queue, otherwise a second hash rx queue using this
> indirection may release it whereas it is still in use by another hash rx
> queue.
Whenever a hash Rx queue is created, it gets to have a ind_tbl either by
mlx5_priv_ind_table_ibv_get() or by mlx5_priv_ind_table_ibv_new(). So, the
refcnt of the ind_tbl is already increased. So, even if other hash RxQ which
have had the ind_tbl releases it, it is safe. That's why I don't think
ind_tbl->refcnt needs to get increased on calling mlx5_priv_hrxq_get(). Makes
sense?
Thanks,
Yongseok
