On Fri, Dec 07, 2018 at 02:51:45PM +0100, Håkon Bugge wrote:
>
>
> > On 3 Dec 2018, at 19:10, Doug Ledford <dledford@xxxxxxxxxx> wrote:
> >
> > On Mon, 2018-12-03 at 17:14 +0100, Håkon Bugge wrote:
> >> Thanks for the review, Doug!
> >
> > You're welcome.
> >
> >> I do not like this series either. Let me suggest; 1) I clean the
> >> commits up and 2), I go back to re-evaluate the use of Port GUID
> >> instead of Node GUID.
> >>
> >> The reason for 2) is that I do not think the ibacm address file will change for most cases, which I presume is a single HCA with one of two ports, and only one address assigned per port.
> >
> > I wouldn't assume one address per port. Every P_Key on a port creates
> > additional addresses to be stored.
>
> Yes, but in another endpoint, as endpoints are designated by Node GUID, Port, and P_Key.
>
> Having looked into proper read/write locking, it will be hairy, because ibacm returns pointers to the (now) dynamic addr_info array. For example, in acm_addr_lookup(), one could add locking such as:
>
> ep = container_of(endpoint, struct acmc_ep, endpoint);
> pthread_rwlock_rdlock(&ep->addr_info_lock);
> for (i = 0; i < ep->nmbr_ep_addrs; i++)
> if (!acm_addr_cmp(&ep->addr_info[i].addr, addr, addr_type)) {
> addr = &ep->addr_info[i].addr;
> break;
> }
>
> pthread_rwlock_unlock(&ep->addr_info_lock);
> return addr;
>
> But this function returns a pointer to what the rw lock protected,
> so after the unlock, the address area pointed to by the return value
> may have been freed and resurrected. So, the lock must be held until
> the returned pointer "has been fully consumed".
Just copy it?
Jason
