>> Perhaps you could update the comment from saying 'map this grant' (which
>> implies doing it NOW as opposed to have done it already), and say:
>>
>> /*
>> .. continue using the grant non-persistently. Note that
>> we mapped it in the earlier loop and the earlier if conditional
>> sets pending_handle(pending_req, i) = map[j].handle.
>> */
>>
>>
>>
>>> }
>>> - pending_handle(pending_req, i) =
>>> - persistent_gnts[i]->handle;
>>> -
>>> - if (ret)
>>> - continue;
>>> - } else {
>>> - pending_handle(pending_req, i) = map[j++].handle;
>>> - bitmap_set(pending_req->unmap_seg, i, 1);
>>> -
>>> - if (ret)
>>> - continue;
>>> + persistent_gnt->gnt = map[j].ref;
>>> + persistent_gnt->handle = map[j].handle;
>>> + persistent_gnt->page = pages[i];
>>
>> Oh boy, that is a confusing. i and j. Keep loosing track which one is which.
>> It lookis right.
>>
>>> + if (add_persistent_gnt(&blkif->persistent_gnts,
>>> + persistent_gnt)) {
>>> + kfree(persistent_gnt);
>>
>> I would also say 'persisten_gnt = NULL' for extra measure of safety
This loop was not completely right, I failed to set the unmap_seg bit if
we tried to map the grant persistently and failed, we would just jump to
next without doing:
bitmap_set(pending_req->unmap_seg, seg_idx, 1);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
