On 11/27/2017 4:03 PM, Don Dutile wrote:
On 11/27/2017 06:25 AM, Leon Romanovsky wrote:
From: Daniel Jurgens danielj@mellanox.com
- if (pps_change && !special_qp) { + if (pps_change && !special_qp && real_qp->qp_sec) { mutex_lock(&real_qp->qp_sec->mutex); new_pps = get_new_pps(real_qp, qp_attr, @@ -600,7 +627,7 @@ int ib_security_modify_qp(struct ib_qp *qp, qp_attr_mask, udata);
- if (pps_change && !special_qp) { + if (pps_change && !special_qp && real_qp->qp_sec) { /* Clean up the lists and free the appropriate * ports_pkeys structure. */
This patch breaks the kernel build on RHEL b/c it generates a warning in the second if (pps_change && !special_qp && real_qp->qp_sec) {} that new_pps may not be assigned. ... build warnings in RHEL kernel == build failure (on x86).
That's b/c the patch adds real_qp->qp_sec to if's conditions, and the compiler cannot determine if real_qp->qp_sec cannot be modified between the first check like it, above, which sets the value of new_pps, and the second check that uses it, because real_qp is passed into the device->modify() function call btwn those two if() check's.
The code needs to do something like this in the first if-check: ..... bool new_pps_gotten = false; ....
if (pps_change && !special_qp && real_qp->qp_sec) { mutex_lock(&real_qp->qp_sec->mutex); new_pps = get_new_pps(real_qp, qp_attr, qp_attr_mask); new_pps_gotten = true; .... } ....
and change the second if check to be:
if (new_pps_gotten) { * Clean up the lists and free the appropriate .....
Thanks Don, I think it's better to initialize new_pps to NULL, vs introducing a new variable. Also, there needs to be a check of new_pps after getting it.