Re: [PATCH] Enable index-pack threading in msysgit.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 19, 2014 at 2:50 PM, Stefan Zager <szager@xxxxxxxxxxxx> wrote:
> On Wed, Mar 19, 2014 at 12:30 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>> On Wed, Mar 19, 2014 at 7:46 AM,  <szager@xxxxxxxxxxxx> wrote:
>>> This adds a Windows implementation of pread.  Note that it is NOT
>>> safe to intersperse calls to read() and pread() on a file
>>> descriptor.  According to the ReadFile spec, using the 'overlapped'
>>> argument should not affect the implicit position pointer of the
>>> descriptor.  Experiments have shown that this is, in fact, a lie.
>>
>> If I understand it correctly, new pread() is added because
>> compat/pread.c does not work because of some other read() in between?
>> Where are those read() (I can only see one in index-pack.c, but there
>> could be some hidden read()..)
>
> I *think* it's the call to fixup_pack_header_footer(), but I'm not 100% sure.
>
> I suppose it would be possible to fix the immediate problem just by
> using one fd per thread, without a new pread implementation.  But it
> seems better overall to have a pread() implementation that is
> thread-safe as long as read() and pread() aren't interspersed; and
> then convert all existing read() calls to pread().  That would be a
> good follow-up patch...

I still don't understand how compat/pread.c does not work with pack_fd
per thread. I don't have Windows to test, but I forced compat/pread.c
on on Linux with similar pack_fd changes and it worked fine, helgrind
only complained about progress.c.

A pread() implementation that is thread-safe with condition sounds
like an invite for trouble later. And I don't think converting read()
to pread() is a good idea. Platforms that rely on pread() will hit
first because of more use of compat/pread.c. read() seeks while
pread() does not, so we have to audit more code..
-- 
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]