Hi Wan-Teh, On Wed, Jul 5, 2017 at 8:08 PM, Wan-Teh Chang <w...@google.com> wrote:
> Hi Ronald, > > On Wed, Jul 5, 2017 at 3:31 PM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: > > Hi Wan-Teh, > > > > On Wed, Jul 5, 2017 at 6:12 PM, Wan-Teh Chang <w...@google.com> wrote: > >> > >> Thank you for all the tsan warning fixes. In the meantime, it would be > >> good to revert 2e664b9c1e73c80aab91070c1eb7676f04bdd12d to avoid > >> confusion. > > > > > > Why? I believe it fixes a subset of the issue. > > Could you explain why it fixes a subset of the issue? From what I remember, I was (before this patch) semi-reliably able to reproduce the issue locally, and it went away after. I've observed the same thing in the relevant fate station, where before the patch, this warning occurred semi-reliably in 3-4 tests per run, and after the patch it sometimes occurs in 1 test per run. I understand this doesn't satisfy you but I currently don't want to dig through the code to figure out why I thought this was a good idea, I have other things that take priority. Reverting the patch will require me to dig through this code also, and I really don't see the gain. Some more thoughts: - I don't think we want to grab a second lock for something trivial like this (e.g. grabbing progress_mutex when copying the debug field in update_context_from_user); - making the debug flags field atomic will be difficult because it's public API, and I don't think we're ready to expose C11 types in our public API; - I suppose you could make a private (inside PerThreadContext, which allows it to be atomic) copy of debug intended for cross-threading purposes and use that here. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
