* Dominik Brodowski <li...@dominikbrodowski.net> wrote:
> > > __sys_mprotect as prefix won't work by the way, as the double-underscore
> > > __sys_
> > > variant is already used in net/* for internal syscall helpers.
> >
> > Ok - then triple underscore - but overall I think it's more confusing.
> >
> > Btw., what was the problem with calling the x86 ptregs wrapper sys_mprotect?
> >
> > The only reason I suggested the __sys_x86_ prefix was because you
> > originally
> > suggested that there's symbol name overlap, but I don't think that's the
> > case
> > within the same kernel build, as the regular non-ptregs prototype:
>
> Indeed, there's no symbol name overlap within the same kernel build, but
> technically different stubs named the same. If that's fine, just drop patch
> 8/8 (including the UML fixup) and things should be fine, with the stub and
> the entry in the syscall table both named sys_mprotect.
Ok, I've dropped patch #8.
> For IA32_EMULATION, we have __sys_ia32_mprotect as stub for the same
> syscall, including this name as entry in syscall_32.tbl.
>
> More problematic is the naming for the compat stubs for IA32_EMAULATION and
> X32, where we have
>
> __compat_sys_ia32_waitid
> __compat_sys_x32_waitid
>
> for example. We *could* rename one of those to compat_sys_waitid() and levae
> the other as-is, but actually I prefer it now how it is.
yeah, this is more symmetric I think.
So right now we have these symbols:
triton:~/tip> grep waitid System.map
ffffffff8105f1e0 t kernel_waitid # common C function (64-bit
kargs)
ffffffff8105f2b0 t SYSC_waitid # 64-bit uaddr args C function
352 bytes
ffffffff8105f410 T sys_waitid # 64-bit-ptregs -> C stub,
32 bytes
ffffffff8105f430 T __sys_ia32_waitid # 32-bit-ptregs -> C stub,
32 bytes
ffffffff8105f450 t C_SYSC_waitid # 32-bit uaddr args C function,
400 bytes
ffffffff8105f5e0 T __compat_sys_ia32_waitid # 32-bit-ptregs -> C stub,
32 bytes
ffffffff8105f600 T __compat_sys_x32_waitid # 64-bit-ptregs -> C stub,
32 bytes
BTW., what is the role of generating __sys_ia32_waitid()? It seems unused when
a
syscall has a compat variant otherwise - like here.
Naming wise the odd thumb out is sys_waitid :-/
I'd argue that we should at minimum name it __sys_waitid:
ffffffff8105f1e0 t kernel_waitid # common C function (64-bit
kargs)
ffffffff8105f2b0 t SYSC_waitid # 64-bit uaddr args C function
ffffffff8105f410 T __sys_waitid # 64-bit-ptregs -> C stub
ffffffff8105f430 T __sys_ia32_waitid # 32-bit-ptregs -> C stub
ffffffff8105f450 t C_SYSC_waitid # 32-bit uaddr args C function
ffffffff8105f5e0 T __compat_sys_ia32_waitid # 32-bit-ptregs -> C stub
ffffffff8105f600 T __compat_sys_x32_waitid # 64-bit-ptregs -> C stub
because that makes it all organized based on the same principle:
{__compat|_}_sys{_ia32|_x32|}_waittid
But arguably there are a whole lot more naming weirdnesses we could fix:
- I find it somewhat confusing that that 'C' in C_SYSC stands not for a C
callign
convention, but for 'COMPAT' - i.e. COMPAT_SYSC would be better.
- Another detail is that why is it called 'SYSC', if the other functions use
the
'sys' prefix? Wouldn't 'SYS' be more consistent?
- If we introduced a prefix for the regular 64-bit system call format as well,
we could have: x64, x32 and ia32.
- And finally, I think the argument format modifiers should be consistently
prefixes - right now they are a mixture of pre- and post-fixes.
I.e. I'd generate the names like this:
__{x64,x32,ia32}[_compat]_sys_waittid()
The fully consistent nomenclature would be someting like this:
ffffffff8105f1e0 t kernel_waitid # common C function (64-bit
kargs)
ffffffff8105f2b0 t SYS_waitid # 64-bit uaddr args C function
ffffffff8105f410 T __x64_sys_waitid # 64-bit-ptregs -> C stub
ffffffff8105f430 T __ia32_sys_waitid # 32-bit-ptregs -> C stub
ffffffff8105f450 t COMPAT_SYS_waitid # 32-bit uaddr args C function
ffffffff8105f5e0 T __ia32_compat_sys_waitid # 32-bit-ptregs -> C stub
ffffffff8105f600 T __x32_compat_sys_waitid # 64-bit-ptregs -> C stub
Looks a lot tidier and a lot more logical, doesn't it?
Makes grepping easier as well, because (case-insensitive) patterns like
'sys_waittid' would identify all the variants.
Personally I'd also do a s/ia32/i32 rename:
ffffffff8105f1e0 t kernel_waitid # common C function (64-bit
kargs)
ffffffff8105f2b0 t SYS_waitid # 64-bit uaddr args C function
ffffffff8105f410 T __x64_sys_waitid # 64-bit-ptregs -> C stub
ffffffff8105f430 T __i32_sys_waitid # 32-bit-ptregs -> C stub
ffffffff8105f450 t COMPAT_SYS_waitid # 32-bit uaddr args C function
ffffffff8105f5e0 T __i32_compat_sys_waitid # 32-bit-ptregs -> C stub
ffffffff8105f600 T __x32_compat_sys_waitid # 64-bit-ptregs -> C stub
... but maybe that's too much ;-)
Thanks,
Ingo
