| Message ID | 20230415161718.80668-2-bugaevc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Let's improve/fix ccty handling | expand |
Applied, thanks! Sergey Bugaev, le sam. 15 avril 2023 19:17:18 +0300, a ecrit: > It is common to have (some of) stdin, stdout and stderr point to the > very same port. We were making the ctty RPCs that _hurd_port2fd () does > for each one of them separately: > > 1. term_getctty () > 2. mach_port_deallocate () > 3. term_open_ctty () > > Instead, let's detect this case and duplicate the ctty port we already > have. This means we do 1 RPC instead of 3 (and create a single protid > on the server side) if the file is our ctty, and no RPCs instead of 1 > if it's not. A clear win! > > Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> > --- > hurd/dtable.c | 46 +++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 39 insertions(+), 7 deletions(-) > > diff --git a/hurd/dtable.c b/hurd/dtable.c > index 5dd04131..41f4d7af 100644 > --- a/hurd/dtable.c > +++ b/hurd/dtable.c > @@ -60,18 +60,50 @@ init_dtable (void) > _hurd_dtable[i] = NULL; > else > { > + int copy; > /* Allocate a new file descriptor structure. */ > struct hurd_fd *new = malloc (sizeof (struct hurd_fd)); > if (new == NULL) > __libc_fatal ("hurd: Can't allocate initial file descriptors\n"); > > - /* Initialize the port cells. */ > - _hurd_port_init (&new->port, MACH_PORT_NULL); > - _hurd_port_init (&new->ctty, MACH_PORT_NULL); > - > - /* Install the port in the descriptor. > - This sets up all the ctty magic. */ > - _hurd_port2fd (new, _hurd_init_dtable[i], 0); > + /* See if this file descriptor is the same as a previous one we have > + already installed. In this case, we can just copy over the same > + ctty port without making any more RPCs. We only check the the > + immediately preceding fd and fd 0 -- this should be enough to > + handle the common cases while not requiring quadratic > + complexity. */ > + if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[i - 1]) > + copy = i - 1; > + else if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[0]) > + copy = 0; > + else > + copy = -1; > + > + if (copy < 0) > + { > + /* Initialize the port cells. */ > + _hurd_port_init (&new->port, MACH_PORT_NULL); > + _hurd_port_init (&new->ctty, MACH_PORT_NULL); > + > + /* Install the port in the descriptor. > + This sets up all the ctty magic. */ > + _hurd_port2fd (new, _hurd_init_dtable[i], 0); > + } > + else > + { > + /* Copy over ctty from the already set up file descriptor that > + contains the same port. We can access the contents of the > + cell without any locking since no one could have seen it > + yet. */ > + mach_port_t ctty = _hurd_dtable[copy]->ctty.port; > + > + if (MACH_PORT_VALID (ctty)) > + __mach_port_mod_refs (__mach_task_self (), ctty, > + MACH_PORT_RIGHT_SEND, +1); > + > + _hurd_port_init (&new->port, _hurd_init_dtable[i]); > + _hurd_port_init (&new->ctty, ctty); > + } > > _hurd_dtable[i] = new; > } > -- > 2.39.2 >
diff --git a/hurd/dtable.c b/hurd/dtable.c index 5dd04131..41f4d7af 100644 --- a/hurd/dtable.c +++ b/hurd/dtable.c @@ -60,18 +60,50 @@ init_dtable (void) _hurd_dtable[i] = NULL; else { + int copy; /* Allocate a new file descriptor structure. */ struct hurd_fd *new = malloc (sizeof (struct hurd_fd)); if (new == NULL) __libc_fatal ("hurd: Can't allocate initial file descriptors\n"); - /* Initialize the port cells. */ - _hurd_port_init (&new->port, MACH_PORT_NULL); - _hurd_port_init (&new->ctty, MACH_PORT_NULL); - - /* Install the port in the descriptor. - This sets up all the ctty magic. */ - _hurd_port2fd (new, _hurd_init_dtable[i], 0); + /* See if this file descriptor is the same as a previous one we have + already installed. In this case, we can just copy over the same + ctty port without making any more RPCs. We only check the the + immediately preceding fd and fd 0 -- this should be enough to + handle the common cases while not requiring quadratic + complexity. */ + if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[i - 1]) + copy = i - 1; + else if (i > 0 && _hurd_init_dtable[i] == _hurd_init_dtable[0]) + copy = 0; + else + copy = -1; + + if (copy < 0) + { + /* Initialize the port cells. */ + _hurd_port_init (&new->port, MACH_PORT_NULL); + _hurd_port_init (&new->ctty, MACH_PORT_NULL); + + /* Install the port in the descriptor. + This sets up all the ctty magic. */ + _hurd_port2fd (new, _hurd_init_dtable[i], 0); + } + else + { + /* Copy over ctty from the already set up file descriptor that + contains the same port. We can access the contents of the + cell without any locking since no one could have seen it + yet. */ + mach_port_t ctty = _hurd_dtable[copy]->ctty.port; + + if (MACH_PORT_VALID (ctty)) + __mach_port_mod_refs (__mach_task_self (), ctty, + MACH_PORT_RIGHT_SEND, +1); + + _hurd_port_init (&new->port, _hurd_init_dtable[i]); + _hurd_port_init (&new->ctty, ctty); + } _hurd_dtable[i] = new; }
It is common to have (some of) stdin, stdout and stderr point to the very same port. We were making the ctty RPCs that _hurd_port2fd () does for each one of them separately: 1. term_getctty () 2. mach_port_deallocate () 3. term_open_ctty () Instead, let's detect this case and duplicate the ctty port we already have. This means we do 1 RPC instead of 3 (and create a single protid on the server side) if the file is our ctty, and no RPCs instead of 1 if it's not. A clear win! Signed-off-by: Sergey Bugaev <bugaevc@gmail.com> --- hurd/dtable.c | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-)