Re: [PATCH v2 0/5] roll back locks in various code paths

On Thu, Mar 01, 2018 at 09:40:20PM +0100, Martin Ågren wrote: > After thinking about it, I tend to agree. That rewrite loses an > indentation level and makes it a bit clearer that we have two steps, > "maybe bail" and "write". But at the cost of duplicating logic -- after > all, those two steps

Re: [PATCH v2 0/5] roll back locks in various code paths

On 1 March 2018 at 20:24, Junio C Hamano wrote: > Jeff King writes: > >> IMHO the result is easier to follow. Except for one case: >> >>> [...] >> >> where I think the logic just ends up repeating itself. > > Yup, this one I also had a bit of trouble with.

Re: [PATCH v2 0/5] roll back locks in various code paths

Jeff King writes: > IMHO the result is easier to follow. Except for one case: > >> -if (active_cache_changed || force_write) { >> -if (newfd < 0) { >> -if (refresh_args.flags & REFRESH_QUIET) >> -exit(128); >> -

Re: [PATCH v2 0/5] roll back locks in various code paths

On Wed, Feb 28, 2018 at 08:58:09PM +0100, Martin Ågren wrote: > > I'll follow up with a patch to > > address the confusing pattern which Peff mentioned and which fooled me > > when I prepared v1. > > Here is such a patch on top of the others. I'm not particularly proud of the > name

Re: [PATCH v2 0/5] roll back locks in various code paths

On Wed, Feb 28, 2018 at 08:07:53PM +0100, Martin Ågren wrote: > This is v2 of my series to always release locks. As before, there's a > conflict with pu, where the correct resolution is to take my version of > the conflicting hunk. > > The only difference to v1 is in patch 3. I'll follow up with

Re: [PATCH v2 0/5] roll back locks in various code paths

On 1 March 2018 at 00:20, Junio C Hamano wrote: > Martin Ågren writes: > >> A further upshot of this patch is that `active_cache_changed`, which is >> defined as `the_index.cache_changed`, now only has a few users left. > > I am undecided if this is a

Re: [PATCH v2 0/5] roll back locks in various code paths

Martin Ågren writes: > A further upshot of this patch is that `active_cache_changed`, which is > defined as `the_index.cache_changed`, now only has a few users left. I am undecided if this is a *good* thing. In a few codepaths where we make a speculative update to the

Re: [PATCH v2 0/5] roll back locks in various code paths

> I'll follow up with a patch to > address the confusing pattern which Peff mentioned and which fooled me > when I prepared v1. Here is such a patch on top of the others. I'm not particularly proud of the name SKIP_IF_UNCHANGED, but don't think it's any worse than, e.g., IGNORE_UNCHANGED or

[PATCH v2 0/5] roll back locks in various code paths

This is v2 of my series to always release locks. As before, there's a conflict with pu, where the correct resolution is to take my version of the conflicting hunk. The only difference to v1 is in patch 3. I'll follow up with a patch to address the confusing pattern which Peff mentioned and which