git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] pack-objects: Use packing_data lock instead of read_mutex

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 23, 2019, 17:51 UTC
Message-ID
<xmqqh8dzcle7.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<CAFOcBz==BunV48U94ZSnc2gmBTOcOpY4XO-dPugKMUVNZGjzZA@mail.gmail.com>
Patrick Hogg <phogg@novamoon.net> writes:
Show 26 quoted lines
> On Tue, Jan 22, 2019 at 5:43 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Patrick Hogg <phogg@novamoon.net> writes:
>>
>> > As I mentioned in the prior thread I think that it will be simpler
>> > to simply use the existing lock in packing_data instead of moving
>> > read_mutex. I can go back to simply moving read_mutex to the
>> > packing_data struct if that that is preferable, though.
>>
>> I'll let others comment on this to show preference between the two
>> approaches.
>>
>> > I also removed the #ifndef NO_PTHREADS in prepare_packing_data around
>> > the initialization of &pdata->lock since I had to upgrade the lock to
>> > a recursive mutex. As far as I can tell init_recursive_mutex (and
>> > pthread_mutex_init for that matter) have that protection already so it
>> > appears to be redundant.
>>
>> If you can defer "I also" to a separate patch, please do so.
>> Keeping the fix alone as small as possible and not tangled with
>> other changes would make it easier for people to cherry-pick the fix
>> to older maintenance tracks if they choose to.
>
> That's a fair point.  To confirm (as I'm rather new to submitting git
> patches), do you mean to submit a two-patch series or to just leave
> out the #ifndef removal altogether for now?
Either would work ;-)
Show 8 quoted lines
> If this does become a two patch series I could simply move the
> read_mutex to packing_data in the first patch and merge the two
> mutexes (and remove the #ifndef) in the second.  That would keep the
> fix alone even smaller (just the first patch) to simplify
> cherry-picking.
>
> (There is also the option of going back to the v1 change and
> correcting the cleanup in the early return.)

Yes, but as I said, I'll let others show their preferences between approaches v1/v2.

Previous: Patrick Hogg
Message 10 of 10 in “pack-objects: Use packing_data lock instead of read_mutex”
  1. pack-objects: Use packing_data lock instead of read_mutexPatrick Hogg, Jan 19, 2019
  2. Duy NguyenJan 21, 2019
  3. Jeff KingJan 22, 2019
  4. Duy NguyenJan 22, 2019
  5. Patrick HoggJan 22, 2019
  6. Elijah NewrenJan 22, 2019
  7. Elijah NewrenJan 22, 2019
  8. Junio C HamanoJan 22, 2019
  9. Patrick HoggJan 22, 2019
  10. Junio C HamanoJan 23, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.