(fixup) [PATCH] forkables: use dynloaded dll's IndexNumber as dirname

Michael Haubenwallner michael.haubenwallner@ssi-schaefer.com
Wed Mar 1 19:18:00 GMT 2017

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.

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!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-forkables-simplify-disabling-via-shm.patch
Type: text/x-patch
Size: 11556 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20170301/49c55fa9/attachment.bin>

More information about the Cygwin-patches mailing list