Subject: bug#14752: sort fails to fork() + execlp(compress_program) if overcommit limit is reached

From: Pádraig Brady <[email-masked]>
Date: Mon, 01 Jul 2013 12:13:52 +0100

On 07/01/2013 11:33 AM, Petros Aggelatos wrote:
> On 07/01/2013 12:52 PM, Pádraig Brady wrote:
>> On 07/01/2013 08:16 AM, Bernhard Voelker wrote: Petros, for
>> completeness, what kernel were you using, and was SELinux enabled?
> I'm running kernel 3.9.7-1-ARCH and SELinux is disabled.
> 
>> Note tmpfs is backed by RAM and swap, so depending on the swapiness
>> settings for the kernel, it will auto migrate to the swap device(s)
>> under RAM pressure.
> I have swap disabled on my system as 16GB or RAM seem more than
> enough. The problem is irrelevant whether it is using tmpfs or not, it
> just happened to trigger the error on my machine as it grew. But one
> can reproduce by disabling overcommit and running a sort with
> compression and -S60%.
> 
>> BTW, -S40% may be the root of the issue. Petros have you tried
>> smaller buffers, which would probably avoid the issue on fork(),
>> but also may take advantage of cache locality.
> I tried using smaller buffers but I needed more temp space which I
> didn't have. If the initial cutting of the file results in more than
> NMERGE chunks then in the worst case it will need:
>   N + NMERGE * P * N / (NMERGE + 1)
> where N is the input size and P is the number of merge-sorts running
> in parallel. But by using -S40% the input file would be split in less
> than 16 chunks so it would be just a single 16-way merge and require
> just N size of temp space.
> 
>> vfork() might be an option here.  One can't rely on it being
>> different to fork(), and it blocks the parent until the exec() in
>> the child, and there are various restrictions on the child, but
>> that might be fine? But I think posix_spawn() is the new
>> standardised equivalent, and I notice the spawn-pipe gnulib module
>> which might be leverged here?
> I did some further research on this after the initial report and
> posix_spawn() seems to me that is the standard way too. I could write
> a patch and test it.

Patches would be gladly accepted!

The higher level spawn-pipe module might not be appropriate,
requiring use of posix_spawn() directly.  In any case I notice
that both are already incorporated in coreutils, through:

  posix-shell
    posix_spawn-internal
    posix_spawn_file_actions_addclose
    posix_spawn_file_actions_addclose-tests
    posix_spawn_file_actions_adddup2
    posix_spawn_file_actions_adddup2-tests
    posix_spawn_file_actions_addopen
    posix_spawn_file_actions_addopen-tests
    posix_spawn_file_actions_destroy
    posix_spawn_file_actions_init
    posix_spawnattr_destroy
    posix_spawnattr_init
    posix_spawnattr_setflags
    posix_spawnattr_setsigmask
    posix_spawnp
    posix_spawnp-tests
  sigaction
    spawn
    spawn-pipe

As a side note I notice that there is no specific cygwin implementation,
though one seems to be available at http://cygwin.com/ml/cygwin/2012-01/msg00032.html

thanks!
Pádraig.