On Tue, Mar 19, 2013 at 3:17 PM, Thomas Rast <trast@xxxxxxxxxxxxxxx> wrote:
>> but the line in question is:
>>
>> if (deepest_delta < delta_obj->delta_depth)
>>
>...
>
> Duy, what was the reasoning why resolve_delta() does not need to hold
> locks when it looks when it looks at deepest_delta? My coffee levels
> aren't up to this task yet. It certainly seems extremely dubious to me,
> as the code uses the global deepest_delta in threaded sections. You can
> probably argue that the load/store is atomic on most(?) platforms, but
> you get no guarantees that deepest_delta at any time in fact holds the
> maximum value of delta_obj->delta_depth.
Now that I have had dinner (and energy restored), the explanation
might be because I missed it. resolve_delta() deals with data in
delta_obj and does not share global state, except this one and
nr_resolved_deltas. The latter is protected. I guess we could protect
this one with a mutex. But only do so when "--verify" is specified
if (stat) {
lock();
if (deepest_delta < delta_obj->delta_depth)
deepest_delta = delta_obj->delta_depth;
unlock();
}
so that we don't need to hold/release lock when index-pack is run.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html