prov/rxm: rxm_mc introduction

[grom72]

rxm_mc added to hide implementation details of collective operations' multicast address

Signed-off-by: Tomasz Gromadzki [email protected]

---

This change is 

[ldorau]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 9 unresolved discussions (waiting on @grom72)

---

-- commits line 5 at r1:
... details of multicast address of collective operations.

Code quote:

  rxm_mc added to hide implementation details of collective operations
  multicast address

---

prov/rxm/src/rxm_ep.c line 1059 at r1 (raw file):

	struct rxm_mc *rxm_mc;
	struct util_coll_mc *coll_mc;
        ofi_ep_lock_acquire(&rxm_ep->util_ep);

You can unify the alignment here and use one tab instead of spaces in this line.

Code quote:

        ofi_ep_lock_acquire(&rxm_ep->util_ep);

---

prov/rxm/src/rxm_ep.c line 1061 at r1 (raw file):

        ofi_ep_lock_acquire(&rxm_ep->util_ep);
	(*req) = rxm_get_coll_buf(rxm_ep);
        ofi_ep_lock_release(&rxm_ep->util_ep);

You can unify the alignment here and use one tab instead of spaces in this line.

Code quote:

        ofi_ep_lock_release(&rxm_ep->util_ep);

---

prov/rxm/src/rxm_ep.c line 1070 at r1 (raw file):

	(*req)->app_context = context;

	rxm_mc = (struct rxm_mc*) ((uintptr_t) *coll_addr) ;

redundant space

Code quote:

) ;

---

prov/rxm/src/rxm_ep.c line 1078 at r1 (raw file):

		coll_mc = container_of(rxm_mc->util_coll_mc_fid,
			struct util_coll_mc, mc_fid);
		*coll_addr = fi_mc_addr(&coll_mc->mc_fid);			

3 redundant tabs at the end of line.

Code quote:

;		·

---

prov/rxm/src/rxm_ep.c line 1081 at r1 (raw file):

		(*coll_ep) = rxm_ep->util_coll_ep;
	}
	else {

	} else {

Code quote:

	}
	else {

---

prov/rxm/src/rxm_ep.c line 1081 at r1 (raw file):

		(*coll_ep) = rxm_ep->util_coll_ep;
	}
	else {

Isn't this still needed here?

} else if (rxm_ep->offload_coll_mask & BIT(coll_op)) {

Can be set

(*coll_ep) = rxm_ep->offload_coll_ep;

without the above condition being true?

Code quote:

else {

---

prov/rxm/src/rxm_ep.c line 1087 at r1 (raw file):

		coll_mc = container_of(rxm_mc->offload_coll_mc_fid,
			struct util_coll_mc, mc_fid);
		*coll_addr = fi_mc_addr(&coll_mc->mc_fid);	

1 redundant tab at the end of line.

Code quote:

);·

---

prov/rxm/src/rxm_ep.c line 1222 at r1 (raw file):

	ssize_t ret;

redundant empty line added

[ldorau]

Reviewed 1 of 4 files at r1.
Reviewable status: all files reviewed, 18 unresolved discussions (waiting on @grom72)

---

prov/rxm/src/rxm_eq.c line 73 at r1 (raw file):

	const struct fi_eq_entry *in_entry = buf;
	struct rxm_mc *mc;
	if (event != FI_JOIN_COMPLETE) {

Missing one empty line in between.

Code quote:

	struct rxm_mc *mc;
	if (event != FI_JOIN_COMPLETE) {

---

prov/rxm/src/rxm_eq.c line 75 at r1 (raw file):

	if (event != FI_JOIN_COMPLETE) {
		return ofi_eq_write(eq_fid, event, buf, len, flags);
	}	

1 redundant tab at the end of line.

Code quote:

	}·

---

prov/rxm/src/rxm_eq.c line 82 at r1 (raw file):

		if (mc->util_coll_join_completed == -1) {
			mc->util_coll_join_completed = 1;
			fi_close (&mc->mc_fid.fid);

1 redundant space in between

Code quote:

fi_close (&mc

---

prov/rxm/src/rxm_eq.c line 91 at r1 (raw file):

		mc->offload_coll_join_completed = 1;
	}
	if (mc->util_coll_join_completed == 1 && 

Missing one empty line in between.

Code quote:

	}
	if (mc->util_coll_join_completed == 1 &&

---

prov/rxm/src/rxm_eq.c line 91 at r1 (raw file):

		mc->offload_coll_join_completed = 1;
	}
	if (mc->util_coll_join_completed == 1 && 

1 redundant space at the end of line.

Code quote:

&&·

---

prov/rxm/src/rxm_eq.c line 92 at r1 (raw file):

	}
	if (mc->util_coll_join_completed == 1 && 
		(mc->offload_coll_join_completed == 1 || 

1 redundant space at the end of line.

Code quote:

||·

---

prov/rxm/src/rxm_eq.c line 92 at r1 (raw file):

	}
	if (mc->util_coll_join_completed == 1 && 
		(mc->offload_coll_join_completed == 1 || 

4 spaces instead of the 2nd tab

Code quote:

		(mc->offload_coll_join_completed == 1 ||
		!mc->offload_coll_mc_fid)) {

---

prov/rxm/src/rxm_eq.c line 99 at r1 (raw file):

		return ofi_eq_write(eq_fid, event, &entry, len, flags);
	}
	return len;

Missing one empty line in between.

Code quote:

	}
	return len;

---

prov/rxm/src/rxm_eq.c line 101 at r1 (raw file):

	return len;
};
static struct fi_ops_eq rxm_eq_ops = {

Missing one empty line in between.

Code quote:

};
static struct fi_ops_eq rxm_eq_ops = {

[ldorau]

Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @grom72)

---

prov/rxm/src/rxm_eq.c line 87 at r1 (raw file):

			mc->util_coll_join_completed = 1;
		}
	} if (in_entry->fid == &mc->offload_coll_mc_fid->fid) {

??? new line or else if ?

Code quote:

} if (in

[grom72]

Reviewed 1 of 4 files at r1, 2 of 3 files at r2, all commit messages.
Reviewable status: 4 of 5 files reviewed, 19 unresolved discussions (waiting on @ldorau)

---

-- commits line 5 at r1:

Previously, ldorau (Lukasz Dorau) wrote…

... details of multicast address of collective operations.

Done.

---

prov/rxm/src/rxm_ep.c line 1059 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

You can unify the alignment here and use one tab instead of spaces in this line.

Done.

---

prov/rxm/src/rxm_ep.c line 1061 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

You can unify the alignment here and use one tab instead of spaces in this line.

Done.

---

prov/rxm/src/rxm_ep.c line 1070 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

redundant space

Done.

---

prov/rxm/src/rxm_ep.c line 1078 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

3 redundant tabs at the end of line.

Done.

---

prov/rxm/src/rxm_ep.c line 1081 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Isn't this still needed here?

} else if (rxm_ep->offload_coll_mask & BIT(coll_op)) {

Can be set

(*coll_ep) = rxm_ep->offload_coll_ep;

without the above condition being true?

It is covered above by !(rxm_ep->offload_coll_mask & BIT(coll_op))

---

prov/rxm/src/rxm_ep.c line 1081 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

	} else {

Done.

---

prov/rxm/src/rxm_ep.c line 1087 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

1 redundant tab at the end of line.

Done.

Code quote:

rxm_ep_broadcast

---

prov/rxm/src/rxm_ep.c line 1222 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

redundant empty line added

Done.

---

prov/rxm/src/rxm_eq.c line 73 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Missing one empty line in between.

Done.

---

prov/rxm/src/rxm_eq.c line 75 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

1 redundant tab at the end of line.

Done.

---

prov/rxm/src/rxm_eq.c line 82 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

1 redundant space in between

Done.

---

prov/rxm/src/rxm_eq.c line 87 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

??? new line or else if ?

Done.

---

prov/rxm/src/rxm_eq.c line 91 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Missing one empty line in between.

Done.

---

prov/rxm/src/rxm_eq.c line 91 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

1 redundant space at the end of line.

Done.

---

prov/rxm/src/rxm_eq.c line 92 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

1 redundant space at the end of line.

Done.

---

prov/rxm/src/rxm_eq.c line 92 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

4 spaces instead of the 2nd tab

Done.

---

prov/rxm/src/rxm_eq.c line 99 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Missing one empty line in between.

Done.

---

prov/rxm/src/rxm_eq.c line 101 at r1 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

Missing one empty line in between.

Done.

[grom72]

Reviewed 1 of 3 files at r2.
Reviewable status: all files reviewed, 19 unresolved discussions (waiting on @ldorau)

[ldorau]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

---

prov/rxm/src/rxm_eq.c line 92 at r2 (raw file):

		mc->offload_coll_join_completed = 1;
	} else {
		assert(0); /* we do not expect any other fid*/

fid */ - add a space at the end

Code quote:

fid*/

[grom72]

Reviewed 1 of 4 files at r3.
Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @ldorau)

---

prov/rxm/src/rxm_eq.c line 92 at r2 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

fid */ - add a space at the end

Done.

[grom72]

Reviewed 1 of 4 files at r1, 3 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ldorau)

[ldorau]

Reviewed 1 of 4 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)

[ldorau]

Reviewed 1 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @grom72)

---

-- commits line 18 at r3:
prov

Code quote:

ptov

---

-- commits line 21 at r3:
completion

Code quote:

compeltion

---

-- commits line 22 at r3:
the same number of references ?

Code quote:

the same references

---

-- commits line 28 at r3:
with the FI_PEER flag

Code quote:

with FI_PEER flag

---

-- commits line 28 at r3:
restoreS

Code quote:

restore

---

-- commits line 28 at r3:
uses it ... ?

Code quote:

uses

---

-- commits line 29 at r3:
reporting the join ...

Code quote:

reporting join

[ldorau]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @grom72)

---

-- commits line 18 at r3:
??? I do not understand the title ...

Code quote:

pass submit

[ldorau]

Reviewed 2 of 4 files at r3.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @grom72)

---

prov/coll/src/coll_coll.c line 932 at r3 (raw file):

		return -FI_ENOSYS;

	if ((flags & FI_PEER)) {

(flags & FI_PEER)

Code quote:

((flags & FI_PEER))

---

prov/coll/src/coll_coll.c line 954 at r3 (raw file):

		return -FI_ENOMEM;

	if ((flags & FI_PEER))

.

Code quote:

((flags & FI_PEER))

[grom72]

Reviewable status: all files reviewed (commit messages unreviewed), 10 unresolved discussions (waiting on @ldorau)

---

-- commits line 18 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

prov

Done.

---

-- commits line 18 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

??? I do not understand the title ...

Done.

---

-- commits line 21 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

completion

Done.

---

-- commits line 22 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

the same number of references ?

Done.

---

-- commits line 28 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

with the FI_PEER flag

Done.

---

-- commits line 28 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

restoreS

Done.

---

-- commits line 28 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

uses it ... ?

Done.

---

-- commits line 29 at r3:

Previously, ldorau (Lukasz Dorau) wrote…

reporting the join ...

Done.

[ldorau]

Reviewed all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72)

---

-- commits line 18 at r4:
provides

Code quote:

provids

[grom72]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @ldorau)

---

-- commits line 18 at r4:

Previously, ldorau (Lukasz Dorau) wrote…

provides

Done.

---

prov/coll/src/coll_coll.c line 932 at r3 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

(flags & FI_PEER)

Done.

---

prov/coll/src/coll_coll.c line 954 at r3 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

.

Done.

[ldorau]

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)

[ldorau]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

---

prov/rxm/src/rxm_ep.c line 468 at r5 (raw file):

			&rxm_mc->offload_coll_mc_fid, &peer_context);
		if (ret) {
			/*mark util_coll_mc to be removed as soon as

/* mark - add one space

Code quote:

/*mark

---

prov/rxm/src/rxm_eq.c line 102 at r5 (raw file):

	if (mc->util_coll_join_completed == 1 &&
	    (mc->offload_coll_join_completed == 1 ||
	     !mc->offload_coll_mc_fid)) {

There is one space too many.

Code quote:

	     !mc->offload_coll_mc_fid)) {

[ldorau]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

[grom72]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ldorau)

---

prov/rxm/src/rxm_eq.c line 102 at r5 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

There is one space too many.

It is intentionally added to mark the bracket that opens the part of the condition

[ldorau]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

---

prov/rxm/src/rxm_eq.c line 102 at r5 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

It is intentionally added to mark the bracket that opens the part of the condition

OK, for me 4 spaces in both lines looks better:

[grom72]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ldorau)

---

prov/rxm/src/rxm_eq.c line 102 at r5 (raw file):

Previously, ldorau (Lukasz Dorau) wrote…

OK, for me 4 spaces in both lines looks better:

What do you think about this version:

	if (mc->util_coll_join_completed == 1
	    && (mc->offload_coll_join_completed == 1
	        || !mc->offload_coll_mc_fid)) {

[ldorau]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)

---

prov/rxm/src/rxm_eq.c line 102 at r5 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

What do you think about this version:

	if (mc->util_coll_join_completed == 1
	    && (mc->offload_coll_join_completed == 1
	        || !mc->offload_coll_mc_fid)) {

It looks good. It can be. (I am afraid our cstyle wouldn't accept it ;-))

[ldorau]

Reviewed all commit messages.
Reviewable status: 14 of 129 files reviewed, all discussions resolved (waiting on @grom72)