This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: (fixup) [PATCH] forkables: use dynloaded dll's IndexNumber as dirname
Hi Corinna,
On 02/23/2017 03:03 PM, Corinna Vinschen wrote:
> Hi Michael,
>
> I'm inclined to promote the "forkables" code to master. I just have a
> few points before we do that:
>
> - Revert bumping of CYGWIN_VERSION_SHARED_DATA. We only have to do that
> if the shared region changes in an incompatible way. But this is just
> adding a member to the end.
Ok.
As long as properly aligned, even int-access should be atomic:
Is it ok to add the new member as 'char' rather than 'int'?
> - I'm looking a bit cross-eyed on the usage of forkables_needs and
> cygwin_shared->prefer_forkable_hardlinks. It seems to me as if the
> split between those two isn't quite right and the fact that both
> share information seems error prone.
>
> IMHO prefer_forkable_hardlinks should actually be the single marker
> for the per-installation state. After startup of the first process it
> should be "unknown" (0) by default. At startup it's set to one of
>
> "disabled" (-1) no NTFS or dir is missing
> "enabled" (+1) NTFS and dir exists
>
> That sets the state once and for all by the first Cygwin process in
> the system.
The initial check now is moved to dll_list::forkable_ntnamesize(),
which is called by dll_list::alloc().
What about the renaming cygwin_shared->prefer_forkable_hardlinks
to cygwin_shared->forkable_hardlink_support?
The new dll_list::forkables_supported() replaces the "unknown","impossible",
"disabled" values. I've thought about inlining into dll_init.h, but that
would require to include "shared_info.h": Is there a specific reason behind
dll_init.h not having any #include right now?
> Consequentially, forkables_needs should only reflect the per-process
> states
>
> "needless"
> "needed"
> "created"
>
> - Shouldn't forkables_needs be renamed to forkables_needed?
I've further simplified and replaced "enum forkables_needs" with
"bool forkables_created", because the "needless" value now is
implemented as "first fork tries without hardlinks". So as soon as
request_forkables() is entered, hardlinks aren't "needless" any more.
> That's all. There are a few minor formatting issues, but they are
> negligible.
Do you prefer another patch series with this patch applied as fixups?
Thanks a lot!
/haubi/
>From bf08d3ae7441f7db23d034cccb9563cee0842fa6 Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Date: Wed, 1 Mar 2017 10:19:37 +0100
Subject: [PATCH] forkables: simplify disabling via shm
* Rename cygwin_shared->prefer_forkable_hardlinks to
forkable_hardlink_support, with values
0 for Unknown, 1 for Supported, -1 for Unsupported.
Upon first dll loaded ever, dll_list::forkable_ntnamesize checks the
/var/run/cygfork directory to both exist and reside on NTFS, setting
cygwin_shared->forkable_hardlink_support accordingly.
* Replace enum forkables_needs by bool forkables_created: Set
to True by request_forkables after creating forkable hardlinks.
---
winsup/cygwin/dll_init.h | 21 ++--
winsup/cygwin/forkable.cc | 185 ++++++++++-----------------------
winsup/cygwin/include/cygwin/version.h | 2 +-
winsup/cygwin/shared.cc | 2 +-
winsup/cygwin/shared_info.h | 4 +-
5 files changed, 65 insertions(+), 149 deletions(-)
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index d598618..7129cee 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -87,17 +87,9 @@ struct dll
class dll_list
{
/* forkables */
- enum
- {
- forkables_unknown,
- forkables_impossible,
- forkables_disabled,
- forkables_needless,
- forkables_needed,
- forkables_created,
- }
- forkables_needs;
+ bool forkables_supported ();
DWORD forkables_dirx_size;
+ bool forkables_created;
PWCHAR forkables_dirx_ntname;
PWCHAR forkables_mutex_name;
HANDLE forkables_mutex;
@@ -160,10 +152,11 @@ public:
void cleanup_forkables ();
bool setup_forkables (bool with_forkables)
{
- if (forkables_needs == forkables_impossible)
- return true; /* short cut to not retry fork */
- /* Once used, always use forkables in current process chain. */
- if (forkables_needs != forkables_unknown)
+ if (!forkables_supported ())
+ return true; /* no need to retry fork */
+ if (forkables_created)
+ /* Once created, use forkables in current
+ process chain on first fork try already. */
with_forkables = true;
if (with_forkables)
request_forkables ();
diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
index ebbed8c..b668d03 100644
--- a/winsup/cygwin/forkable.cc
+++ b/winsup/cygwin/forkable.cc
@@ -501,17 +501,56 @@ dll::create_forkable ()
return false;
}
+bool
+dll_list::forkables_supported ()
+{
+ return cygwin_shared->forkable_hardlink_support >= 0;
+}
+
/* return the number of characters necessary to store one forkable name */
size_t
dll_list::forkable_ntnamesize (dll_type type, PCWCHAR fullntname, PCWCHAR modname)
{
/* per process, this is the first forkables-method ever called */
- if (forkables_needs == forkables_unknown &&
- !cygwin_shared->prefer_forkable_hardlinks)
- forkables_needs = forkables_impossible; /* short cut */
+ if (cygwin_shared->forkable_hardlink_support == 0) /* Unknown */
+ {
+ /* check existence of forkables dir */
+ PWCHAR pbuf = nt_max_path_buf ();
+ for (namepart const *part = forkable_nameparts; part->text; ++part)
+ {
+ if (part->textfunc)
+ pbuf += part->textfunc (pbuf, -1);
+ else
+ pbuf += __small_swprintf (pbuf, L"%W", part->text);
+ if (part->mutex_from_dir)
+ break; /* up to first mutex-naming dir */
+ }
+ pbuf = nt_max_path_buf ();
- if (forkables_needs == forkables_impossible)
- return 0;
+ UNICODE_STRING fn;
+ RtlInitUnicodeString (&fn, pbuf);
+
+ HANDLE dh = INVALID_HANDLE_VALUE;
+ fs_info fsi;
+ if (fsi.update (&fn, NULL) &&
+/* FIXME: !fsi.is_readonly () && */
+ fsi.is_ntfs ())
+ dh = ntopenfile (pbuf, NULL, FILE_DIRECTORY_FILE);
+ if (dh != INVALID_HANDLE_VALUE)
+ {
+ cygwin_shared->forkable_hardlink_support = 1; /* Yes */
+ NtClose (dh);
+ debug_printf ("enabled");
+ }
+ else
+ {
+ cygwin_shared->forkable_hardlink_support = -1; /* No */
+ debug_printf ("disabled, missing or not on NTFS %W", fn.Buffer);
+ }
+ }
+
+ if (!forkables_supported ())
+ return 0;
if (!forkables_dirx_size)
{
@@ -560,9 +599,6 @@ dll_list::forkable_ntnamesize (dll_type type, PCWCHAR fullntname, PCWCHAR modnam
void
dll_list::prepare_forkables_nomination ()
{
- if (!forkables_dirx_ntname)
- return;
-
dll *d = &dlls.start;
while ((d = d->next))
stat_real_file_once (d); /* uses nt_max_path_buf () */
@@ -627,106 +663,19 @@ dll_list::prepare_forkables_nomination ()
void
dll_list::update_forkables_needs ()
{
- dll *d;
-
- if (forkables_needs == forkables_unknown)
- {
- /* check if filesystem of forkables dir is NTFS */
- PWCHAR pbuf = nt_max_path_buf ();
- for (namepart const *part = forkable_nameparts; part->text; ++part)
- {
- if (part->mutex_from_dir)
- break; /* leading non-mutex-naming dirs, must exist anyway */
- if (part->textfunc)
- pbuf += part->textfunc (pbuf, -1);
- else
- pbuf += __small_swprintf (pbuf, L"%W", part->text);
- }
-
- UNICODE_STRING fn;
- RtlInitUnicodeString (&fn, nt_max_path_buf ());
-
- fs_info fsi;
- if (fsi.update (&fn, NULL) &&
-/* FIXME: !fsi.is_readonly () && */
- fsi.is_ntfs ())
- forkables_needs = forkables_disabled; /* check directory itself */
- else
- {
- debug_printf ("impossible, not on NTFS %W", fn.Buffer);
- forkables_needs = forkables_impossible;
- cygwin_shared->prefer_forkable_hardlinks = 0;
- }
- }
-
- if (forkables_needs == forkables_impossible)
- return; /* we have not created any hardlink, nothing to clean up */
-
- if (forkables_needs == forkables_disabled ||
- forkables_needs == forkables_needless ||
- forkables_needs == forkables_created)
- {
- /* (re-)check existence of forkables dir */
- PWCHAR pbuf = nt_max_path_buf ();
- for (namepart const *part = forkable_nameparts; part->text; ++part)
- {
- if (part->textfunc)
- pbuf += part->textfunc (pbuf, -1);
- else
- pbuf += __small_swprintf (pbuf, L"%W", part->text);
- if (part->mutex_from_dir)
- break; /* up to first mutex-naming dir */
- }
- pbuf = nt_max_path_buf ();
-
- HANDLE dh = ntopenfile (pbuf, NULL, FILE_DIRECTORY_FILE);
- if (dh != INVALID_HANDLE_VALUE)
- {
- NtClose (dh);
- if (forkables_needs == forkables_disabled)
- forkables_needs = forkables_needless;
- }
- else if (forkables_needs != forkables_disabled)
- {
- debug_printf ("disabled, disappearing %W", pbuf);
- close_mutex ();
- denominate_forkables ();
- forkables_needs = forkables_disabled;
- }
- else
- debug_printf ("disabled, missing %W", pbuf);
- }
-
- if (forkables_needs == forkables_disabled)
- return;
-
- if (forkables_needs == forkables_created)
+ if (forkables_created)
{
/* already have created hardlinks in this process, ... */
- forkables_needs = forkables_needless;
- d = &start;
+ dll *d = &start;
while ((d = d->next) != NULL)
if (d->forkable_ntname && !*d->forkable_ntname)
{
/* ... but another dll was loaded since last fork */
debug_printf ("needed, since last fork loaded %W", d->ntname);
- forkables_needs = forkables_needed;
+ forkables_created = false;
break;
}
}
-
- if (forkables_needs > forkables_needless)
- return; /* no need to check anything else */
-
- if (forkables_needs != forkables_needless)
- {
- /* paranoia */
- system_printf ("WARNING: invalid forkables_needs value %d",
- forkables_needs);
- return;
- }
-
- forkables_needs = forkables_needed;
}
/* Create the nominated forkable hardlinks and directories as necessary,
@@ -958,7 +907,7 @@ dll_list::close_mutex ()
void
dll_list::cleanup_forkables ()
{
- if (!forkables_dirx_ntname)
+ if (!forkables_supported ())
return;
bool locked = close_mutex ();
@@ -1034,58 +983,32 @@ dll_list::set_forkables_inheritance (bool inherit)
void
dll_list::request_forkables ()
{
- if (!forkables_dirx_ntname)
+ if (!forkables_supported ())
return;
- /* Even on forkables_impossible, keep the number of open handles
- stable across the fork, and close them when releasing only. */
prepare_forkables_nomination ();
update_forkables_needs ();
set_forkables_inheritance (true);
- if (forkables_needs == forkables_disabled)
- {
- /* we do not support (re-)enabling on the fly */
- forkables_needs = forkables_impossible;
- cygwin_shared->prefer_forkable_hardlinks = 0;
- }
-
- if (forkables_needs <= forkables_needless)
- return;
+ if (forkables_created)
+ return; /* nothing new to create */
dll *d = &start;
while ((d = d->next))
d->nominate_forkable (forkables_dirx_ntname);
- bool updated = update_forkables ();
-
- if (!updated)
- forkables_needs = forkables_needless;
- else
- forkables_needs = forkables_created;
+ if (update_forkables ())
+ forkables_created = true;
}
void
dll_list::release_forkables ()
{
- if (!forkables_dirx_ntname)
+ if (!forkables_supported ())
return;
set_forkables_inheritance (false);
-
- if (forkables_needs == forkables_impossible)
- {
- cleanup_forkables ();
-
- dll *d = &start;
- while ((d = d->next))
- d->forkable_ntname = NULL;
-
- cfree (forkables_dirx_ntname);
- forkables_dirx_ntname = NULL;
- forkables_mutex_name = NULL;
- }
}
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index f449805..0da3d37 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -482,7 +482,7 @@ details. */
regions. It is incremented when incompatible changes are made to the shared
memory region *or* to any named shared mutexes, semaphores, etc. */
-#define CYGWIN_VERSION_SHARED_DATA 6
+#define CYGWIN_VERSION_SHARED_DATA 5
/* An identifier used in the names used to create shared objects. The full
names include the CYGWIN_VERSION_SHARED_DATA version as well as this
diff --git a/winsup/cygwin/shared.cc b/winsup/cygwin/shared.cc
index 4d754d0..06684f4 100644
--- a/winsup/cygwin/shared.cc
+++ b/winsup/cygwin/shared.cc
@@ -328,7 +328,7 @@ shared_info::initialize ()
init_obcaseinsensitive (); /* Initialize obcaseinsensitive */
tty.init (); /* Initialize tty table */
mt.initialize (); /* Initialize shared tape information */
- prefer_forkable_hardlinks = 1; /* Yes */
+ forkable_hardlink_support = 0; /* 0: Unknown, 1: Yes, -1: No */
/* Defer debug output printing the installation root and installation key
up to this point. Debug output except for system_printf requires
the global shared memory to exist. */
diff --git a/winsup/cygwin/shared_info.h b/winsup/cygwin/shared_info.h
index ad7c8f4..fcc53d7 100644
--- a/winsup/cygwin/shared_info.h
+++ b/winsup/cygwin/shared_info.h
@@ -32,7 +32,7 @@ public:
/* Data accessible to all tasks */
-#define CURR_SHARED_MAGIC 0xbc5d6a9cU
+#define CURR_SHARED_MAGIC 0x72c39e6fU
#define USER_VERSION 1
@@ -48,7 +48,7 @@ class shared_info
LONG last_used_bindresvport;
DWORD obcaseinsensitive;
mtinfo mt;
- char prefer_forkable_hardlinks; /* single byte access always is atomic */
+ char forkable_hardlink_support; /* single byte access always is atomic */
void initialize ();
void init_obcaseinsensitive ();
--
2.8.3