Each struct scheduler currently doubles as both a scheduler 
backend's static vtable (name, opt_name, sched_id and every 
function pointer) and the per-cpupool runtime object that 
scheduler_alloc() allocates. Because these are the same type, 
scheduler_alloc() memcpy()s the entire vtable into a fresh heap 
allocation for every cpupool it creates. With N cpupools running 
the same scheduler, this duplicates N copies of identical function 
pointers and identifying fields that never differ between 
instances - the only fields that are genuinely per-cpupool are 
sched_data and cpupool.

This series splits the vtable out into its own type, struct 
sched_ops, so it can be shared by every cpupool using a given 
scheduler instead of copied per cpupool. struct scheduler is left 
holding only what is actually per-instance: a pointer to the 
shared sched_ops, plus sched_data and cpupool.

The series is structured as introduce/migrate/remove, so that 
every commit builds and boots on its own:

  - The first patch adds struct sched_ops, REGISTER_SCHED_OPS(), 
    and a sched_ops_array[] alongside the existing schedulers[], 
    extending every lookup path (scheduler_alloc(), 
    sched_get_by_name(), scheduler_init()) to search both arrays. 
    This is purely additive - no scheduler uses it yet. 

  - The next five patches each migrate one scheduler backend 
    (credit, credit2, rtds, arinc653, null) from struct scheduler 
    to struct sched_ops. Each is small, mechanical, and 
    independently bisectable, with no behavioral difference, since 
    scheduler_alloc() builds an identical runtime struct scheduler 
    regardless of which array a match is found in. 

  - The final patch removes the old schedulers[] and 
    REGISTER_SCHEDULER() path now that nothing uses it, shrinks 
    struct scheduler down to { ops, sched_data, cpupool }, and 
    updates every accessor in private.h accordingly.

Furkan Caliskan (7):
  xen/sched: introduce struct sched_ops as a shared scheduler vtable
  xen/sched: credit: migrate to new sched_ops
  xen/sched: credit2: migrate to new sched_ops
  xen/sched: rtds: migrate to new sched_ops
  xen/sched: arinc653: migrate to new sched_ops
  xen/sched: null: migrate to new sched_ops
  xen/sched: remove old scheduler registration, shrink struct scheduler

 xen/arch/arm/xen.lds.S      |   2 +-
 xen/arch/ppc/xen.lds.S      |   2 +-
 xen/arch/riscv/xen.lds.S    |   2 +-
 xen/arch/x86/xen.lds.S      |   2 +-
 xen/common/sched/arinc653.c |  11 +---
 xen/common/sched/core.c     |  80 ++++++++++++++++-------------
 xen/common/sched/cpupool.c  |   6 +--
 xen/common/sched/credit.c   |   5 +-
 xen/common/sched/credit2.c  |   5 +-
 xen/common/sched/null.c     |   5 +-
 xen/common/sched/private.h  | 100 +++++++++++++++++++-----------------
 xen/common/sched/rt.c       |   5 +-
 xen/include/xen/xen.lds.h   |   8 +--
 13 files changed, 116 insertions(+), 117 deletions(-)

-- 
2.34.1

On 03.08.26 07:06, Furkan Caliskan wrote:
> Each struct scheduler currently doubles as both a scheduler
> backend's static vtable (name, opt_name, sched_id and every
> function pointer) and the per-cpupool runtime object that
> scheduler_alloc() allocates. Because these are the same type,
> scheduler_alloc() memcpy()s the entire vtable into a fresh heap
> allocation for every cpupool it creates. With N cpupools running
> the same scheduler, this duplicates N copies of identical function
> pointers and identifying fields that never differ between
> instances - the only fields that are genuinely per-cpupool are
> sched_data and cpupool.
> 
> This series splits the vtable out into its own type, struct
> sched_ops, so it can be shared by every cpupool using a given
> scheduler instead of copied per cpupool. struct scheduler is left
> holding only what is actually per-instance: a pointer to the
> shared sched_ops, plus sched_data and cpupool.
> 
> The series is structured as introduce/migrate/remove, so that
> every commit builds and boots on its own:
> 
>    - The first patch adds struct sched_ops, REGISTER_SCHED_OPS(),
>      and a sched_ops_array[] alongside the existing schedulers[],
>      extending every lookup path (scheduler_alloc(),
>      sched_get_by_name(), scheduler_init()) to search both arrays.
>      This is purely additive - no scheduler uses it yet.
> 
>    - The next five patches each migrate one scheduler backend
>      (credit, credit2, rtds, arinc653, null) from struct scheduler
>      to struct sched_ops. Each is small, mechanical, and
>      independently bisectable, with no behavioral difference, since
>      scheduler_alloc() builds an identical runtime struct scheduler
>      regardless of which array a match is found in.
> 
>    - The final patch removes the old schedulers[] and
>      REGISTER_SCHEDULER() path now that nothing uses it, shrinks
>      struct scheduler down to { ops, sched_data, cpupool }, and
>      updates every accessor in private.h accordingly.
> 
> Furkan Caliskan (7):
>    xen/sched: introduce struct sched_ops as a shared scheduler vtable
>    xen/sched: credit: migrate to new sched_ops
>    xen/sched: credit2: migrate to new sched_ops
>    xen/sched: rtds: migrate to new sched_ops
>    xen/sched: arinc653: migrate to new sched_ops
>    xen/sched: null: migrate to new sched_ops
>    xen/sched: remove old scheduler registration, shrink struct scheduler
> 
>   xen/arch/arm/xen.lds.S      |   2 +-
>   xen/arch/ppc/xen.lds.S      |   2 +-
>   xen/arch/riscv/xen.lds.S    |   2 +-
>   xen/arch/x86/xen.lds.S      |   2 +-
>   xen/common/sched/arinc653.c |  11 +---
>   xen/common/sched/core.c     |  80 ++++++++++++++++-------------
>   xen/common/sched/cpupool.c  |   6 +--
>   xen/common/sched/credit.c   |   5 +-
>   xen/common/sched/credit2.c  |   5 +-
>   xen/common/sched/null.c     |   5 +-
>   xen/common/sched/private.h  | 100 +++++++++++++++++++-----------------
>   xen/common/sched/rt.c       |   5 +-
>   xen/include/xen/xen.lds.h   |   8 +--
>   13 files changed, 116 insertions(+), 117 deletions(-)
> 

You have a series here which is adding 116 lines and removing 117.

Patch 7 alone is removing 248 lines while adding 67 lines.

So in the end there is a single patch in this series which has more code
churn than the complete series when added in one go.

IOW: making this just a single patch would be easier to review than the
last patch alone, let alone all the temporary modifications which would
be gone when merging all patches into one. And with that you could even
drop some of the renaming you did (e.g. in the linker file), making the
diff even smaller.

I agree with the overall goal, but I'm sparing my time doing a thorough
review of the series in this shape.


Juergen

Hi Jürgen,

On 8/3/26 13:34, Jürgen Groß wrote:
> On 03.08.26 07:06, Furkan Caliskan wrote:
>> Each struct scheduler currently doubles as both a scheduler
>> backend's static vtable (name, opt_name, sched_id and every
>> function pointer) and the per-cpupool runtime object that
>> scheduler_alloc() allocates. Because these are the same type,
>> scheduler_alloc() memcpy()s the entire vtable into a fresh heap
>> allocation for every cpupool it creates. With N cpupools running
>> the same scheduler, this duplicates N copies of identical function
>> pointers and identifying fields that never differ between
>> instances - the only fields that are genuinely per-cpupool are
>> sched_data and cpupool.
>>
>> This series splits the vtable out into its own type, struct
>> sched_ops, so it can be shared by every cpupool using a given
>> scheduler instead of copied per cpupool. struct scheduler is left
>> holding only what is actually per-instance: a pointer to the
>> shared sched_ops, plus sched_data and cpupool.
>>
>> The series is structured as introduce/migrate/remove, so that
>> every commit builds and boots on its own:
>>
>>    - The first patch adds struct sched_ops, REGISTER_SCHED_OPS(),
>>      and a sched_ops_array[] alongside the existing schedulers[],
>>      extending every lookup path (scheduler_alloc(),
>>      sched_get_by_name(), scheduler_init()) to search both arrays.
>>      This is purely additive - no scheduler uses it yet.
>>
>>    - The next five patches each migrate one scheduler backend
>>      (credit, credit2, rtds, arinc653, null) from struct scheduler
>>      to struct sched_ops. Each is small, mechanical, and
>>      independently bisectable, with no behavioral difference, since
>>      scheduler_alloc() builds an identical runtime struct scheduler
>>      regardless of which array a match is found in.
>>
>>    - The final patch removes the old schedulers[] and
>>      REGISTER_SCHEDULER() path now that nothing uses it, shrinks
>>      struct scheduler down to { ops, sched_data, cpupool }, and
>>      updates every accessor in private.h accordingly.
>>
>> Furkan Caliskan (7):
>>    xen/sched: introduce struct sched_ops as a shared scheduler vtable
>>    xen/sched: credit: migrate to new sched_ops
>>    xen/sched: credit2: migrate to new sched_ops
>>    xen/sched: rtds: migrate to new sched_ops
>>    xen/sched: arinc653: migrate to new sched_ops
>>    xen/sched: null: migrate to new sched_ops
>>    xen/sched: remove old scheduler registration, shrink struct scheduler
>>
>>   xen/arch/arm/xen.lds.S      |   2 +-
>>   xen/arch/ppc/xen.lds.S      |   2 +-
>>   xen/arch/riscv/xen.lds.S    |   2 +-
>>   xen/arch/x86/xen.lds.S      |   2 +-
>>   xen/common/sched/arinc653.c |  11 +---
>>   xen/common/sched/core.c     |  80 ++++++++++++++++-------------
>>   xen/common/sched/cpupool.c  |   6 +--
>>   xen/common/sched/credit.c   |   5 +-
>>   xen/common/sched/credit2.c  |   5 +-
>>   xen/common/sched/null.c     |   5 +-
>>   xen/common/sched/private.h  | 100 +++++++++++++++++++-----------------
>>   xen/common/sched/rt.c       |   5 +-
>>   xen/include/xen/xen.lds.h   |   8 +--
>>   13 files changed, 116 insertions(+), 117 deletions(-)
>>
> 
> You have a series here which is adding 116 lines and removing 117.
> 
> Patch 7 alone is removing 248 lines while adding 67 lines.
> 
> So in the end there is a single patch in this series which has more code
> churn than the complete series when added in one go.
> 
> IOW: making this just a single patch would be easier to review than the
> last patch alone, let alone all the temporary modifications which would
> be gone when merging all patches into one. And with that you could even
> drop some of the renaming you did (e.g. in the linker file), making the
> diff even smaller.
> 
> I agree with the overall goal, but I'm sparing my time doing a thorough
> review of the series in this shape.
> 
> 
> Juergen

My first instinct was actually to just send this as one patch. I split 
it up because I wanted each scheduler's conversion to be its own small, 
bisectable commit, but you're right that it's not worth it here. 

I will squash it into a single patch and resend. But I would still like 
to rename SCHEDULER_ARRAY to SCHED_OPS_ARRAY in the per-arch linker files 
and other related variable names in other files, since they now hold 
sched_ops entries rather than struct scheduler ones, and I think the name 
should reflect that. 

Thanks for the feedback,

Furkan Caliskan

On 03.08.26 13:12, Furkan Çalışkan wrote:
> Hi Jürgen,
> 
> On 8/3/26 13:34, Jürgen Groß wrote:
>> On 03.08.26 07:06, Furkan Caliskan wrote:
>>> Each struct scheduler currently doubles as both a scheduler
>>> backend's static vtable (name, opt_name, sched_id and every
>>> function pointer) and the per-cpupool runtime object that
>>> scheduler_alloc() allocates. Because these are the same type,
>>> scheduler_alloc() memcpy()s the entire vtable into a fresh heap
>>> allocation for every cpupool it creates. With N cpupools running
>>> the same scheduler, this duplicates N copies of identical function
>>> pointers and identifying fields that never differ between
>>> instances - the only fields that are genuinely per-cpupool are
>>> sched_data and cpupool.
>>>
>>> This series splits the vtable out into its own type, struct
>>> sched_ops, so it can be shared by every cpupool using a given
>>> scheduler instead of copied per cpupool. struct scheduler is left
>>> holding only what is actually per-instance: a pointer to the
>>> shared sched_ops, plus sched_data and cpupool.
>>>
>>> The series is structured as introduce/migrate/remove, so that
>>> every commit builds and boots on its own:
>>>
>>>     - The first patch adds struct sched_ops, REGISTER_SCHED_OPS(),
>>>       and a sched_ops_array[] alongside the existing schedulers[],
>>>       extending every lookup path (scheduler_alloc(),
>>>       sched_get_by_name(), scheduler_init()) to search both arrays.
>>>       This is purely additive - no scheduler uses it yet.
>>>
>>>     - The next five patches each migrate one scheduler backend
>>>       (credit, credit2, rtds, arinc653, null) from struct scheduler
>>>       to struct sched_ops. Each is small, mechanical, and
>>>       independently bisectable, with no behavioral difference, since
>>>       scheduler_alloc() builds an identical runtime struct scheduler
>>>       regardless of which array a match is found in.
>>>
>>>     - The final patch removes the old schedulers[] and
>>>       REGISTER_SCHEDULER() path now that nothing uses it, shrinks
>>>       struct scheduler down to { ops, sched_data, cpupool }, and
>>>       updates every accessor in private.h accordingly.
>>>
>>> Furkan Caliskan (7):
>>>     xen/sched: introduce struct sched_ops as a shared scheduler vtable
>>>     xen/sched: credit: migrate to new sched_ops
>>>     xen/sched: credit2: migrate to new sched_ops
>>>     xen/sched: rtds: migrate to new sched_ops
>>>     xen/sched: arinc653: migrate to new sched_ops
>>>     xen/sched: null: migrate to new sched_ops
>>>     xen/sched: remove old scheduler registration, shrink struct scheduler
>>>
>>>    xen/arch/arm/xen.lds.S      |   2 +-
>>>    xen/arch/ppc/xen.lds.S      |   2 +-
>>>    xen/arch/riscv/xen.lds.S    |   2 +-
>>>    xen/arch/x86/xen.lds.S      |   2 +-
>>>    xen/common/sched/arinc653.c |  11 +---
>>>    xen/common/sched/core.c     |  80 ++++++++++++++++-------------
>>>    xen/common/sched/cpupool.c  |   6 +--
>>>    xen/common/sched/credit.c   |   5 +-
>>>    xen/common/sched/credit2.c  |   5 +-
>>>    xen/common/sched/null.c     |   5 +-
>>>    xen/common/sched/private.h  | 100 +++++++++++++++++++-----------------
>>>    xen/common/sched/rt.c       |   5 +-
>>>    xen/include/xen/xen.lds.h   |   8 +--
>>>    13 files changed, 116 insertions(+), 117 deletions(-)
>>>
>>
>> You have a series here which is adding 116 lines and removing 117.
>>
>> Patch 7 alone is removing 248 lines while adding 67 lines.
>>
>> So in the end there is a single patch in this series which has more code
>> churn than the complete series when added in one go.
>>
>> IOW: making this just a single patch would be easier to review than the
>> last patch alone, let alone all the temporary modifications which would
>> be gone when merging all patches into one. And with that you could even
>> drop some of the renaming you did (e.g. in the linker file), making the
>> diff even smaller.
>>
>> I agree with the overall goal, but I'm sparing my time doing a thorough
>> review of the series in this shape.
>>
>>
>> Juergen
> 
> My first instinct was actually to just send this as one patch. I split
> it up because I wanted each scheduler's conversion to be its own small,
> bisectable commit, but you're right that it's not worth it here.
> 
> I will squash it into a single patch and resend. But I would still like
> to rename SCHEDULER_ARRAY to SCHED_OPS_ARRAY in the per-arch linker files
> and other related variable names in other files, since they now hold
> sched_ops entries rather than struct scheduler ones, and I think the name
> should reflect that.

Now THIS could be done in a separate patch.


Juergen