bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 0/1] Let's improve/fix ccty handling


From: Sergey Bugaev
Subject: [PATCH 0/1] Let's improve/fix ccty handling
Date: Sat, 15 Apr 2023 19:17:17 +0300

Hello!

Here are some assorted thoughts on cttys (controlling terminals).

Running rpctrace on any simple program, I see this:

<snip>
  13<--33(pid1354)->term_getctty () = 0    4<--39(pid1354)
task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0
  13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument)
  13<--33(pid1354)->term_getctty () = 0    4<--39(pid1354)
task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0
  13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument)
  13<--33(pid1354)->term_getctty () = 0    4<--39(pid1354)
task16(pid1354)->mach_port_deallocate (pn{ 10}) = 0
  13<--33(pid1354)->term_open_ctty (0 0) = 0x40000016 (Invalid argument)
<snip>

This happens inside init_dtable () -> _hurd_port2fd (). We can notice
that:

1. stdin, stdout, stderr all refer to the same port (/dev/ttyp1 in my
   case), and yet glibc tries to set up ctty handling for each fd
   separately, making way too many RPCs. This happens each time a
   process starts up => not cool!
2. ctty setup actually fails with EINVAL! -- even though this is a
   valid tty.

I'm attaching a patch to fix #1, so now we get this:

<snip>
  13<--33(pid1255)->term_getctty () = 0    4<--39(pid1255)
task16(pid1255)->mach_port_deallocate (pn{ 10}) = 0
  13<--33(pid1255)->term_open_ctty (0 0) = 0x40000016 (Invalid argument)
<snip>

Much better, but still, what's that error? Well, this is what
S_term_open_ctty () checks:

  if (pid <= 0 || pgrp <= 0)
    {
      return EINVAL;
    }

so it makes sense that it errors out if we pass pid = 0, pgrp = 0. But
why are we passing that? Looking at the code, it does

__term_open_ctty (dport, _hurd_pid, _hurd_pgrp, &ctty);

so clearly at this point our _hurd_pid / _hurd_pgrp are not initialized
yet, and indeed the proc_getpids_request / proc_getpgrp_request RPCs
happen much later in the trace.

I think commit 1ccbb9258eed0f667edf459a28ba23a805549b36
"hurd: Notify the proc server later during initialization" was the one
that has broken things here (I guess ctty working is not a part of the
testsuite? :). The commit makes sense, in that we want to start doing
signals late; but apparently we need to fetch our pid/pgrp early. This
itself would be fine, but I'm not sure how to implement it. Fetching
pid/pgrp is done in init_pids (in hurdpid.c), which is attached to the
_hurd_proc_subinit hook (and is the only thing attached there).

Do I understand it right that this hooks system is libc-internal? --
or can user programs add their own functions to the hooks? (Looks like
it works through the linker-defined start/end section symbols, which
wouldn't work across DSOs, which is why I think it must be
libc-internal.) If it is internal to libc, we're not bounded by
compatibility concerns, and have more liberty in shuffling things
around.

_hurd_proc_subinit is defined as "Hook for things which should be
initialized as soon as the proc server is available." If that is
accurate, surely we don't need to have started signals and told the
proc server our argv location for it to be "available"? Can we just
run this hook sooner, namely in _hurd_init, once we set
_hurd_portarray but before we do RUN_RELHOOK (_hurd_subinit, ())?
_hurd_subinit is the hook init_dtable is attached to (and again,
there's nothing else on it).

Note: it's important to avoid calling init_pids multiple times during
startup, because each call does two RPCs, and we don't want any extra
ones!


Now for another thing: glibc also makes these term_getctty RPCs when
they are clearly pointless, such as when loading the locale archive:

11<--36(pid1403)->dir_lookup ("usr/lib/locale/locale-archive" 4194305 0) = 0 1 
""    48<--43(pid1403)
48<--43(pid1403)->term_getctty () = 0xfffffed1 ((ipc/mig) bad request message 
ID)

We should be probably using O_IGNORE_CTTY (which makes it not do that)
in more places where we expect to open a regular file, not a terminal.
But how do we do that, considering O_IGNORE_CTTY is Hurd-specific? We
could, for instance, do

#ifndef O_IGNORE_CTTY
#define O_IGNORE_CTTY 0
#endif

in each .c file where we'd like to use it. Or maybe there's some
internal version of fcntl.h for the Linux port where we could just

#define O_IGNORE_CTTY 0

without exposing it to user code?

Sergey



reply via email to

[Prev in Thread] Current Thread [Next in Thread]