On 12/9/2015 10:47 PM, Leon Romanovsky wrote:
> On Wed, Dec 09, 2015 at 10:42:19AM -0800, Nelson Escobar wrote:
>> - if (usnic_vnic_res_free_cnt(vnic, type) < cnt || cnt < 1 || !owner)
>> + if (usnic_vnic_res_free_cnt(vnic, type) < cnt || cnt < 0 || !owner)
> Before this change you returned EINVAL if no free_cnt were available,
> now you will continue. is this behaviour expected?
Yes. If cnt is 0, then no resources are being requested, so it is OK if
there are no resources available.
>
>> return ERR_PTR(-EINVAL);
>>
>> ret = kzalloc(sizeof(*ret), GFP_ATOMIC);
>> @@ -247,26 +247,28 @@ usnic_vnic_get_resources(struct usnic_vnic *vnic, enum
>> usnic_vnic_res_type type,
>> return ERR_PTR(-ENOMEM);
>> }
>>
>> - ret->res = kzalloc(sizeof(*(ret->res))*cnt, GFP_ATOMIC);
>> - if (!ret->res) {
>> - usnic_err("Failed to allocate resources for %s. Out of
>> memory\n",
>> - usnic_vnic_pci_name(vnic));
>> - kfree(ret);
>> - return ERR_PTR(-ENOMEM);
>> - }
>> + if (cnt > 0) {
>> + ret->res = kcalloc(cnt, sizeof(*(ret->res)), GFP_ATOMIC);
>> + if (!ret->res) {
>> + usnic_err("Failed to allocate resources for %s. Out of
>> memory\n",
>> + usnic_vnic_pci_name(vnic));
> You don't need to print OOM messages, failure in memory allocation very hard
> to miss.
OOM messages are hard to miss, but this message is already in upstream
and outside the scope of this patch.
>> + kfree(ret);
>> + return ERR_PTR(-ENOMEM);
>> + }
>>
>> - spin_lock(&vnic->res_lock);
>> - src = &vnic->chunks[type];
>> - for (i = 0; i < src->cnt && ret->cnt < cnt; i++) {
>> - res = src->res[i];
>> - if (!res->owner) {
>> - src->free_cnt--;
>> - res->owner = owner;
>> - ret->res[ret->cnt++] = res;
>> + spin_lock(&vnic->res_lock);
>> + src = &vnic->chunks[type];
>> + for (i = 0; i < src->cnt && ret->cnt < cnt; i++) {
>> + res = src->res[i];
>> + if (!res->owner) {
>> + src->free_cnt--;
> It will be negative, because of skip usnic_vnic_res_free_cnt check
> before.
We are inside the 'if (cnt > 0)' clause here, so the previous
usnic_vnic_res_free_cnt check wasn't skipped.
>> + res->owner = owner;
>> + ret->res[ret->cnt++] = res;
>> + }
>> }
>> - }
>>
>> - spin_unlock(&vnic->res_lock);
>> + spin_unlock(&vnic->res_lock);
>> + }
>> ret->type = type;
>> ret->vnic = vnic;
>> WARN_ON(ret->cnt != cnt);
>> @@ -281,14 +283,16 @@ void usnic_vnic_put_resources(struct
>> usnic_vnic_res_chunk *chunk)
>> int i;
>> struct usnic_vnic *vnic = chunk->vnic;
>>
>> - spin_lock(&vnic->res_lock);
>> - while ((i = --chunk->cnt) >= 0) {
>> - res = chunk->res[i];
>> - chunk->res[i] = NULL;
>> - res->owner = NULL;
>> - vnic->chunks[res->type].free_cnt++;
>> + if (chunk->cnt > 0) {
>> + spin_lock(&vnic->res_lock);
>> + while ((i = --chunk->cnt) >= 0) {
>> + res = chunk->res[i];
>> + chunk->res[i] = NULL;
>> + res->owner = NULL;
>> + vnic->chunks[res->type].free_cnt++;
>> + }
>> + spin_unlock(&vnic->res_lock);
>> }
>> - spin_unlock(&vnic->res_lock);
>>
>> kfree(chunk->res);
>> kfree(chunk);
>> --
>> 2.4.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
