Re: [PATCH] fs/exec.c: Add fast path for ENOENT on PATH search before allocating mm

From: Mateusz Guzik
Date: Thu Nov 09 2023 - 07:21:09 EST


On 11/9/23, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> Mateusz Guzik <mjguzik@xxxxxxxxx> writes:
>> sched_exec causes migration only for only few % of execs in the bench,
>> but when it does happen there is tons of overhead elsewhere.
>>
>> I expect real programs which get past execve will be prone to
>> migrating anyway, regardless of what sched_exec is doing.
>>
>> That is to say, while sched_exec buggering off here would be nice, I
>> think for real-world wins the thing to investigate is the overhead
>> which comes from migration to begin with.
>
> I have a vague memory that the idea is that there is a point during exec
> when it should be much less expensive than normal to allow migration
> between cpus because all of the old state has gone away.
>
> Assuming that is the rationale, if we are getting lock contention
> then either there is a global lock in there, or there is the potential
> to pick a less expensive location within exec.
>

Given the commit below I think the term "migration cost" is overloaded here.

By migration cost in my previous mail I meant the immediate cost
(stop_one_cpu and so on), but also the aftermath -- for example tlb
flushes on another CPU when tearing down your now-defunct mm after you
switched.

For testing purposes I verified commenting out sched_exec and not
using taskset still gives me about 9.5k ops/s.

I 100% agree should the task be moved between NUMA domains, it makes
sense to do it when it has the smallest footprint. I don't know what
the original patch did, the current code just picks a CPU and migrates
to it, regardless of NUMA considerations. I will note that the goal
would still be achieved by comparing domains and doing nothing if they
match.

I think this would be nice to fix, but it is definitely not a big
deal. I guess the question is to Peter Zijlstra if this sounds
reasonable.

> Just to confirm my memory I dug a little deeper and I found the original
> commit that added sched_exec (in tglx's git tree of the bit keeper
> history).
>
> commit f01419fd6d4e5b32fef19d206bc3550cc04567a9
> Author: Martin J. Bligh <mbligh@xxxxxxxxxxx>
> Date: Wed Jan 15 19:46:10 2003 -0800
>
> [PATCH] (2/3) Initial load balancing
>
> Patch from Michael Hohnbaum
>
> This adds a hook, sched_balance_exec(), to the exec code, to make it
> place the exec'ed task on the least loaded queue. We have less state
> to move at exec time than fork time, so this is the cheapest point
> to cross-node migrate. Experience in Dynix/PTX and testing on Linux
> has confirmed that this is the cheapest time to move tasks between
> nodes.
>
> It also macro-wraps changes to nr_running, to allow us to keep track of
> per-node nr_running as well. Again, no impact on non-NUMA machines.
>
>
> Eric
>
>


--
Mateusz Guzik <mjguzik gmail.com>