Re: [PATCH v2 4/8] NTB: ntb_pingpong: Choose doorbells based on port number

[Logan Gunthorpe]

On 24/07/18 11:26 AM, Allen Hubbe wrote:
> On Mon, Jul 23, 2018 at 12:08 PM Logan Gunthorpe <logang@xxxxxxxxxxxx> wrote:
>> I don't think you'll ever have a case where two peers have the same
>> index, as the index is really an abstract concept the hardware doesn't
>> really know about.
> 
> That is the point of index, that there should never be two peers with
> the same index, and also that the range of index values is bounded.

Yes, I think we are making the same point.

> Port numbers are problematic, so I'm worried about the change to use
> port number in the client drivers instead of using index.  For
> example, this change assumes that the index value is < bits per long
> long, because the value is used in BIT_ULL(port number).

Huh?, I'm not making that change... We still use the index to refer to
the peer, but the resource we are using is based on the port number,
just as it was before my changes. Perhaps you can point out in my
patches what change you are worried about?

> Maybe I'm missing something...  In the crosslink case, there is
> another doorbell register on the other side of the crosslink.  Whether
> to use the nearby or via-crosslink doorbell depends on the peer
> index... making assumptions about the hw driver, but is that about
> right? 

Not really. Given that we know there are only two peers, we always use
the other side's doorbell register. You'd only use the nearby doorbell
register if you wanted to trigger your own interrupt -- that would be
weird and we don't really have the API sophistication to do that.

If we wanted to support multiple peers with some number in crosslink
then we'd need to revamp things _significantly_. In this case we'd have
multiple doorbell registers which each apply to a different subset of
peers. This gets _very_ complicated and hurts my head. But as I said,
I'm not trying to add new functionality for multi-peer crosslink or
anything like that. I'm just trying to fix the 2 crosslink peer case so
it works like it did when it was originally merged.

Logan