sza...@chromium.org writes:
> 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.
>
> To accomodate that fact, this change also incorporates:
>
> http://article.gmane.org/gmane.comp.version-control.git/196042
>
> ... which gives each index-pack thread its own file descriptor.
> ---
Sign-off?
The new "per-thread file descriptors to the same thing" in a generic
codepath is a bit of eyesore. For index-pack, keeping as many file
descritors open to the current pack as the worker threads are should
not be too bad, but could we have some comment next to the field
definition please (e.g. "Windows emulation of pread() needs separate
fd per thread; see $URL for details" or something)?
> builtin/index-pack.c | 21 ++++++++++++++++-----
> compat/mingw.c | 31 ++++++++++++++++++++++++++++++-
> compat/mingw.h | 3 +++
> config.mak.uname | 1 -
> 4 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 2f37a38..c02dd4c 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -51,6 +51,7 @@ struct thread_local {
> #endif
> struct base_data *base_cache;
> size_t base_cache_used;
> + int pack_fd;
> };
>
> /*
> @@ -91,7 +92,8 @@ static off_t consumed_bytes;
> static unsigned deepest_delta;
> static git_SHA_CTX input_ctx;
> static uint32_t input_crc32;
> -static int input_fd, output_fd, pack_fd;
> +static const char *curr_pack;
> +static int input_fd, output_fd;
>
> #ifndef NO_PTHREADS
>
> @@ -134,6 +136,7 @@ static inline void unlock_mutex(pthread_mutex_t *mutex)
> */
> static void init_thread(void)
> {
> + int i;
> init_recursive_mutex(&read_mutex);
> pthread_mutex_init(&counter_mutex, NULL);
> pthread_mutex_init(&work_mutex, NULL);
> @@ -141,11 +144,17 @@ static void init_thread(void)
> pthread_mutex_init(&deepest_delta_mutex, NULL);
> pthread_key_create(&key, NULL);
> thread_data = xcalloc(nr_threads, sizeof(*thread_data));
> + for (i = 0; i < nr_threads; i++) {
> + thread_data[i].pack_fd = open(curr_pack, O_RDONLY);
> + if (thread_data[i].pack_fd == -1)
> + die_errno("unable to open %s", curr_pack);
> + }
> threads_active = 1;
> }
>
> static void cleanup_thread(void)
> {
> + int i;
> if (!threads_active)
> return;
> threads_active = 0;
> @@ -155,6 +164,8 @@ static void cleanup_thread(void)
> if (show_stat)
> pthread_mutex_destroy(&deepest_delta_mutex);
> pthread_key_delete(key);
> + for (i = 0; i < nr_threads; i++)
> + close(thread_data[i].pack_fd);
> free(thread_data);
> }
>
> @@ -288,13 +299,13 @@ static const char *open_pack_file(const char *pack_name)
> output_fd = open(pack_name, O_CREAT|O_EXCL|O_RDWR,
> 0600);
> if (output_fd < 0)
> die_errno(_("unable to create '%s'"), pack_name);
> - pack_fd = output_fd;
> + nothread_data.pack_fd = output_fd;
> } else {
> input_fd = open(pack_name, O_RDONLY);
> if (input_fd < 0)
> die_errno(_("cannot open packfile '%s'"), pack_name);
> output_fd = -1;
> - pack_fd = input_fd;
> + nothread_data.pack_fd = input_fd;
> }
> git_SHA1_Init(&input_ctx);
> return pack_name;
> @@ -542,7 +553,7 @@ static void *unpack_data(struct object_entry *obj,
>
> do {
> ssize_t n = (len < 64*1024) ? len : 64*1024;
> - n = pread(pack_fd, inbuf, n, from);
> + n = pread(get_thread_data()->pack_fd, inbuf, n, from);
> if (n < 0)
> die_errno(_("cannot pread pack file"));
> if (!n)
> @@ -1490,7 +1501,7 @@ static void show_pack_info(int stat_only)
> int cmd_index_pack(int argc, const char **argv, const char *prefix)
> {
> int i, fix_thin_pack = 0, verify = 0, stat_only = 0;
> - const char *curr_pack, *curr_index;
> + const char *curr_index;
> const char *index_name = NULL, *pack_name = NULL;
> const char *keep_name = NULL, *keep_msg = NULL;
> char *index_name_buf = NULL, *keep_name_buf = NULL;
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 383cafe..6cc85d6 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -329,7 +329,36 @@ int mingw_mkdir(const char *path, int mode)
> return ret;
> }
>
> -int mingw_open (const char *filename, int oflags, ...)
> +
> +ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset)
> +{
> + HANDLE hand = (HANDLE)_get_osfhandle(fd);
> + if (hand == INVALID_HANDLE_VALUE) {
> + errno = EBADF;
> + return -1;
> + }
> +
> + LARGE_INTEGER offset_value;
> + offset_value.QuadPart = offset;
> +
> + DWORD bytes_read = 0;
> + OVERLAPPED overlapped = {0};
> + overlapped.Offset = offset_value.LowPart;
> + overlapped.OffsetHigh = offset_value.HighPart;
> + BOOL result = ReadFile(hand, buf, count, &bytes_read, &overlapped);
> +
> + ssize_t ret = bytes_read;
> +
> + if (!result && GetLastError() != ERROR_HANDLE_EOF)
> + {
> + errno = err_win_to_posix(GetLastError());
> + ret = -1;
> + }
> +
> + return ret;
> +}
> +
> +int mingw_open(const char *filename, int oflags, ...)
> {
> va_list args;
> unsigned mode;
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 08b83fe..377ba50 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -174,6 +174,9 @@ int mingw_unlink(const char *pathname);
> int mingw_rmdir(const char *path);
> #define rmdir mingw_rmdir
>
> +ssize_t mingw_pread(int fd, void *buf, size_t count, off64_t offset);
> +#define pread mingw_pread
> +
> int mingw_open (const char *filename, int oflags, ...);
> #define open mingw_open
>
> diff --git a/config.mak.uname b/config.mak.uname
> index e8acc39..b405524 100644
> --- a/config.mak.uname
> +++ b/config.mak.uname
> @@ -474,7 +474,6 @@ ifeq ($(uname_S),NONSTOP_KERNEL)
> endif
> ifneq (,$(findstring MINGW,$(uname_S)))
> pathsep = ;
> - NO_PREAD = YesPlease
> NEEDS_CRYPTO_WITH_SSL = YesPlease
> NO_LIBGEN_H = YesPlease
> NO_POLL = YesPlease
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
