[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#14752: sort fails to fork() + execlp(compress_program) if overcommit
From: |
Pádraig Brady |
Subject: |
bug#14752: sort fails to fork() + execlp(compress_program) if overcommit limit is reached |
Date: |
Mon, 01 Jul 2013 12:13:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
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.