Re: [PATCH v3 2/2] linux-user: call set/getscheduler set/getparam directly

[Laurent Vivier]

Le 23/12/2021 à 07:47, Tonis Tiigi a écrit :
> There seems to be difference in syscall and libc definition of these
> methods and therefore musl does not implement them (1e21e78bf7). Call
> syscall directly to ensure the behavior of the libc of user application,
> not the libc that was used to build QEMU.
>
> Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
> ---
>   linux-user/syscall.c      | 40 ++++++++++++++++++++++++++-------------
>   linux-user/syscall_defs.h |  4 ++++
>   2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 2f5a0fac5a..8c03a52a36 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -345,6 +345,17 @@ _syscall4(int, sys_sched_getattr, pid_t, pid, struct 
> target_sched_attr *, attr,
>   #define __NR_sys_sched_setattr __NR_sched_setattr
>   _syscall3(int, sys_sched_setattr, pid_t, pid, struct target_sched_attr *, 
> attr,
>             unsigned int, flags);
> +#define __NR_sys_sched_getscheduler __NR_sched_getscheduler
> +_syscall1(int, sys_sched_getscheduler, pid_t, pid);
> +#define __NR_sys_sched_setscheduler __NR_sched_setscheduler
> +_syscall3(int, sys_sched_setscheduler, pid_t, pid, int, policy,
> +          const struct target_sched_param *, param);
> +#define __NR_sys_sched_getparam __NR_sched_getparam
> +_syscall2(int, sys_sched_getparam, pid_t, pid,
> +          struct target_sched_param *, param);
> +#define __NR_sys_sched_setparam __NR_sched_setparam
> +_syscall2(int, sys_sched_setparam, pid_t, pid,
> +          const struct target_sched_param *, param);
>   #define __NR_sys_getcpu __NR_getcpu
>   _syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache);
>   _syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd,
> @@ -10555,30 +10566,32 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>           return ret;
>       case TARGET_NR_sched_setparam:
>           {
> -            struct sched_param *target_schp;
> -            struct sched_param schp;
> +            struct target_sched_param *target_schp;
> +            struct target_sched_param schp;

You need to keep sched_param for schp as it is used with host syscall.

>   
>               if (arg2 == 0) {
>                   return -TARGET_EINVAL;
>               }
> -            if (!lock_user_struct(VERIFY_READ, target_schp, arg2, 1))
> +            if (!lock_user_struct(VERIFY_READ, target_schp, arg2, 1)) {
>                   return -TARGET_EFAULT;
> +            }
>               schp.sched_priority = tswap32(target_schp->sched_priority);
>               unlock_user_struct(target_schp, arg2, 0);
> -            return get_errno(sched_setparam(arg1, &schp));
> +            return get_errno(sys_sched_setparam(arg1, &schp));
>           }
>       case TARGET_NR_sched_getparam:
>           {
> -            struct sched_param *target_schp;
> -            struct sched_param schp;
> +            struct target_sched_param *target_schp;
> +            struct target_sched_param schp;

You need to keep sched_param for schp as it is used with host syscall.

>   
>               if (arg2 == 0) {
>                   return -TARGET_EINVAL;
>               }
> -            ret = get_errno(sched_getparam(arg1, &schp));
> +            ret = get_errno(sys_sched_getparam(arg1, &schp));
>               if (!is_error(ret)) {
> -                if (!lock_user_struct(VERIFY_WRITE, target_schp, arg2, 0))
> +                if (!lock_user_struct(VERIFY_WRITE, target_schp, arg2, 0)) {
>                       return -TARGET_EFAULT;
> +                }
>                   target_schp->sched_priority = tswap32(schp.sched_priority);
>                   unlock_user_struct(target_schp, arg2, 1);
>               }
> @@ -10586,19 +10599,20 @@ static abi_long do_syscall1(void *cpu_env, int num, 
> abi_long arg1,
>           return ret;
>       case TARGET_NR_sched_setscheduler:
>           {
> -            struct sched_param *target_schp;
> -            struct sched_param schp;
> +            struct target_sched_param *target_schp;
> +            struct target_sched_param schp;

You need to keep sched_param for schp as it is used with host syscall.

>               if (arg3 == 0) {
>                   return -TARGET_EINVAL;
>               }
> -            if (!lock_user_struct(VERIFY_READ, target_schp, arg3, 1))
> +            if (!lock_user_struct(VERIFY_READ, target_schp, arg3, 1)) {
>                   return -TARGET_EFAULT;
> +            }
>               schp.sched_priority = tswap32(target_schp->sched_priority);
>               unlock_user_struct(target_schp, arg3, 0);
> -            return get_errno(sched_setscheduler(arg1, arg2, &schp));
> +            return get_errno(sys_sched_setscheduler(arg1, arg2, &schp));
>           }
>       case TARGET_NR_sched_getscheduler:
> -        return get_errno(sched_getscheduler(arg1));
> +        return get_errno(sys_sched_getscheduler(arg1));
>       case TARGET_NR_sched_getattr:
>           {
>               struct target_sched_attr *target_scha;
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 310d6ce8ad..28b9fe9a47 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2928,4 +2928,8 @@ struct target_sched_attr {
>       abi_uint sched_util_max;
>   };
>   
> +struct target_sched_param {
> +    abi_int sched_priority;
> +};
> +
>   #endif

Sorry, I missed these problem in my previous review.

Thanks,
Laurent