On Mon, Jul 23, 2018 at 08:49:59PM +0200, Duy Nguyen wrote: > On Mon, Jul 23, 2018 at 8:38 PM Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > I will have to study the thread dispatch code more to have a better > > answer, unfortunately. > > Well.. I thought I would need this weekend for this, but a quick look > and ll_find_deltas() suggests that what we're doing is safe. At least > you and Jeff are way to familiar with the delta window concept in > pack-objects. So in multithread mode, we have a big list of all > objects, then the list is cut in N sublists and each is owned entirely > by one thread. Each thread then can slide a window over this sublist > to search for the best delta. > > Because of this partitioning, if trg_entry is being processed now, it > will not be accessed by any other thread. It's owned by this thread > and will be accessed again as src_entry when the next entry becomes > the delta target in the same thread. As long as we don't access > objects of another thread (and my v1 violates this) we should be ok. Yes, that matches my knowledge of how this all works. And if it didn't, I think the code would have been racy even _before_ your patches. The only thing that this pack->delta_size approach is changing is that managing that array needs to happen under lock, because it touches the whole list. And as long as we back-fill it from any arbitrary e->delta_size_, that means that touching e->delta_size_ needs to be done under lock. That's why I think keeping the individual valid flag in each entry makes the most sense. Then whenever we overflow a particular e->delta_size_, we don't have to care about anybody else's size. Which I think is what your v2 patch is doing. -Peff
