Re: [PATCH 2/3] Save handlers between calls to ioctl

[olafBuddenhagen]

Hi,

On Wed, Aug 26, 2009 at 04:44:58PM +0200, Carl Fredrik Hammar wrote:

> diff --git a/hurd/Makefile b/hurd/Makefile
> index 4ad5128..768f93a 100644
> --- a/hurd/Makefile
> +++ b/hurd/Makefile
> @@ -68,7 +68,7 @@ sig = hurdsig hurdfault siginfo hurd-raise preempt-sig \
>  dtable       = dtable port2fd new-fd alloc-fd intern-fd \
>         getdport openport \
>         fd-close fd-read fd-write hurdioctl ctty-input ctty-output \
> -       fd-ioctl-call
> +       fd-ioctl-call fd-ioctl-cleanup
>  inlines = $(inline-headers:%.h=%-inlines)
>  distribute = hurdstartup.h hurdfault.h hurdhost.h sysvshm.h \
>            faultexc.defs intr-rpc.defs intr-rpc.h intr-msg.h Notes
> diff --git a/hurd/dtable.c b/hurd/dtable.c
> index 125345e..bf5a9c1 100644
> --- a/hurd/dtable.c
> +++ b/hurd/dtable.c
> @@ -1,4 +1,5 @@
> -/* Copyright (C) 1991,92,93,94,95,96,97,99 Free Software Foundation, Inc.
> +/* Copyright (C) 1991,92,93,94,95,96,97,99,2009
> +     Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -70,6 +71,11 @@ init_dtable (void)
>         _hurd_port_init (&new->port, MACH_PORT_NULL);
>         _hurd_port_init (&new->ctty, MACH_PORT_NULL);
>  
> +       /* Initialize the ioctl handler.  */
> +       new->ioctl_handler = NULL;
> +       new->ioctl_handler_map = NULL;
> +       new->ioctl_handler_users = NULL;
> +
>         /* Install the port in the descriptor.
>            This sets up all the ctty magic.  */
>         _hurd_port2fd (new, _hurd_init_dtable[i], 0);
> diff --git a/hurd/fd-close.c b/hurd/fd-close.c
> index f497d75..f3d0aa5 100644
> --- a/hurd/fd-close.c
> +++ b/hurd/fd-close.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1994, 1995, 1997 Free Software Foundation, Inc.
> +/* Copyright (C) 1994, 1995, 1997, 2009  Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -17,6 +17,7 @@
>     02111-1307 USA.  */
>  
>  #include <hurd/fd.h>
> +#include <dlfcn.h>
>  
>  error_t
>  _hurd_fd_close (struct hurd_fd *fd)
> @@ -33,10 +34,20 @@ _hurd_fd_close (struct hurd_fd *fd)
>      }
>    else
>      {
> +      /* Clear the descriptor's ioctl handler, and close its liker map.  */
> +      if (fd->ioctl_handler_map != NULL
> +       && _hurd_userlink_clear (&fd->ioctl_handler_users))
> +     __libc_dlclose (fd->ioctl_handler_map);
> +
> +      fd->ioctl_handler = NULL;
> +      fd->ioctl_handler_map = NULL;
> +      fd->ioctl_handler_users = NULL;
> +
>        /* Clear the descriptor's port cells.
>        This deallocates the ports if noone else is still using them.  */
>        _hurd_port_set (&fd->ctty, MACH_PORT_NULL);
>        _hurd_port_locked_set (&fd->port, MACH_PORT_NULL);
> +
>        err = 0;
>      }
>  
> diff --git a/hurd/fd-ioctl-call.c b/hurd/fd-ioctl-call.c
> index c5a41e8..873a5ca 100644
> --- a/hurd/fd-ioctl-call.c
> +++ b/hurd/fd-ioctl-call.c
> @@ -61,30 +61,112 @@ load_ioctl_handler (io_t port, void **map)
>  }
>  
>  
> -/* Call D's ioctl handler, loading it from the underlying port if
> -   necessary.  Arguments are the same as ioctl handlers.  */
> +/* Load and install D's ioctl handler.  D should be locked and CRIT should
> +   point to a critical section lock.  CRIT is unlocked whenever D is
> +   unlocked and a new lock is returned in CRIT if D needs to be relocked.
> +   D is unlocked while the handler is loaded.  If the underlying port
> +   of D changes while it's unlocked the operation is retried with the
> +   new port.  This is repeated until the port remains unchanged, or
> +   if it becomes null D remains unlocked and EBADF is returned.  D is
> +   relocked and the ioctl handler is installed.  If the load failed,
> +   a dummy ioctl handler is installed with a null linker map.  */
>  
> -int
> -_hurd_fd_call_ioctl_handler (int fd, int request, void *arg)
> +static error_t
> +install_ioctl_handler (struct hurd_fd *d, void **crit)

You haven't used --patience when generating this patch, have you?...
Makes this part rather ugly -- I refuse to read it :-)

>  {
> +  struct hurd_userlink ulink;
> +  io_t port;
>    ioctl_handler_t ioctl_handler;
>    void *ioctl_handler_map;
> -  int result;
>    error_t err;
>  
> -  /* Avoid spurious "may be used uninitialized" warning.  */
> -  ioctl_handler_map = NULL;
> +  while (!d->ioctl_handler)
> +    {
> +      port = _hurd_port_locked_get (&d->port, &ulink);
> +      _hurd_critical_section_unlock (*crit);
> +      if (port == MACH_PORT_NULL)
> +     /* ULINK isn't linked when port is null.  */
> +     return EBADF;
> +
> +      /* Avoid spurious "may be used uninitialized" warning.  */
> +      ioctl_handler_map = NULL;
> +
> +      err = load_ioctl_handler (port, &ioctl_handler_map);
> +      if (!err && ioctl_handler_map)
> +     ioctl_handler = __libc_dlsym (ioctl_handler_map,
> +                                   "hurd_ioctl_handler");
> +      if (err || !ioctl_handler_map || !ioctl_handler)
> +     ioctl_handler = _hurd_dummy_ioctl_handler;
> +
> +      *crit = _hurd_critical_section_lock ();
> +      __spin_lock (&d->port.lock);
> +
> +      if (_hurd_userlink_unlink (&ulink))
> +     __mach_port_deallocate (__mach_task_self (), port);
> +      else if (port == d->port.port)
> +     {
> +       d->ioctl_handler = ioctl_handler;
> +       if (ioctl_handler_map)
> +         d->ioctl_handler_map = ioctl_handler_map;
> +     }
> +    }
> +
> +  return 0;
> +}
> +
> +
> +/* Call D's ioctl handler, loading it from the underlying port if
> +   necessary.  Arguments are the same as ioctl handlers, except for
> +   CRIT in which a new lock is returned.  */
> +
> +error_t
> +_hurd_fd_call_ioctl_handler (int fd, struct hurd_fd *d, void **crit,
> +                          int request, void *arg, int *result)
> +{
> +  struct hurd_userlink ioctl_ulink;
> +  void *ioctl_handler_map;
> +  error_t err;
> +
> +  err = install_ioctl_handler (d, crit);
> +  if (err)
> +   {
> +     *result = __hurd_fail (err);
> +     return 0;
> +   }
>  
> -  err = HURD_DPORT_USE (fd, load_ioctl_handler (port, &ioctl_handler_map));
> -  if (!err && ioctl_handler_map)
> -    ioctl_handler = __libc_dlsym (ioctl_handler_map, "hurd_ioctl_handler");
> -  if (err || !ioctl_handler_map || !ioctl_handler)
> -    ioctl_handler = _hurd_dummy_ioctl_handler;
> +  ioctl_handler_map = d->ioctl_handler_map;
> +  if (ioctl_handler_map)
> +    {
> +      ioctl_ulink.cleanup = &_hurd_fd_ioctl_handler_map_cleanup;
> +      ioctl_ulink.cleanup_data = (void *) ioctl_handler_map;
> +      _hurd_userlink_link (&d->ioctl_handler_users, &ioctl_ulink);
> +    }
>  
> -  result = (*ioctl_handler) (fd, request, arg);
> +  err = (*d->ioctl_handler) (fd, d, *crit, request, arg, result);
>  
>    if (ioctl_handler_map)
> -    __libc_dlclose (ioctl_handler_map);
> +    /* Unlink if from D's icotl user list.  Relock D for the duration
> +       if the handler unlocked it.  */
> +    {
> +      void dealloc (void)
> +     {
> +       if (_hurd_userlink_unlink (&ioctl_ulink))
> +         __libc_dlclose (ioctl_handler_map);
> +     }
>  
> -  return result;
> +      if (err)
> +        dealloc ();
> +      else
> +     {
> +       void *crit = _hurd_critical_section_lock ();
> +       __spin_lock (&d->port.lock);
> +
> +       dealloc ();
> +
> +       __spin_unlock (&d->port.lock);
> +       _hurd_critical_section_unlock (crit);
> +     }
> +    }
> +
> +  return err;
>  }
> diff --git a/hurd/fd-ioctl-cleanup.c b/hurd/fd-ioctl-cleanup.c
> new file mode 100644
> index 0000000..df6e432
> --- /dev/null
> +++ b/hurd/fd-ioctl-cleanup.c
> @@ -0,0 +1,31 @@
> +/* Cleanup function for FD ioctl handler users who longjmp.
> +   Copyright (C) 2009 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, write to the Free
> +   Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> +   Boston, MA 02110-1301, USA.  */
> +
> +#include <hurd/fd.h>
> +#include <dlfcn.h>
> +
> +/* The last user of the ioctl handler linker map DATA is now doing
> +   `longjmp (ENV, VAL)', and this will unwind the frame of that last user.
> +    Close the linker map he will never get back to using.  */
> +
> +void
> +_hurd_fd_ioctl_handler_map_cleanup (void *data, jmp_buf env, int val)
> +{
> +  __libc_dlclose (data);
> +}
> diff --git a/hurd/hurd/fd.h b/hurd/hurd/fd.h
> index 6ea9ad6..2137b6b 100644
> --- a/hurd/hurd/fd.h
> +++ b/hurd/hurd/fd.h
> @@ -30,6 +30,28 @@
>  #include <sys/socket.h>
>  
>  
> +struct hurd_fd;  /* Forward declaration.  */
> +
> +/* Type of handler function, called by ioctl to do its entire job.
> +   D should be _hurd_dtable[FD], and it should be valid and locked,
> +   CRIT should be a critical section lock.  If the ioctl is handled 0
> +   is returned, D and CRIT is unlocked, and the return value of ioctl
> +   is returned in RESULT, and any error is returned in ERRNO.  If not
> +   handled ENOTTY, or any error that prevents the ioctl from being
> +   handled, is returned, leaving D and CRIT locked.
> +
> +   D must be locked until a ioctl handler is found that can handle the
> +   ioctl, otherwise it may get a new underlying port with a different
> +   ioctl handler, which might affect the choice of ioctl handler.
> +
> +   Two errors are returned so it's possible to distinguish between an
> +   unhandled ioctl and a RPC that returns ENOTTY.

Why can't you use errno, as in the previous version?...

> +   The latter can't be
> +   handled by simply relocking D, since D's port may change while it's
> +   unlocked.  */
> +typedef error_t (*ioctl_handler_t) (int fd, struct hurd_fd *d, void *crit,
> +                                 int request, void *arg, int *result);
> +
> +
>  /* Structure representing a file descriptor.  */
>  
>  struct hurd_fd
> @@ -41,6 +63,16 @@ struct hurd_fd
>         the same io object but which never returns EBACKGROUND; when not,
>         this is nil.  */
>      struct hurd_port ctty;
> +
> +    /* Server provided handler for ioctls, and map for the dynamically
> +       loaded module it resides in as returned by `dlopen'.  The
> +       module is loaded on the first call to `ioctl', before this
> +       IOCTL_HANDLER is NULL, and if the load fails it is set
> +       to `_hurd_dummy_ioctl_handler'.  Their values are specific to PORT
> +       and should be invalidated whenever `port' is changed.  */
> +    ioctl_handler_t ioctl_handler;
> +    void *ioctl_handler_map;
> +    struct hurd_userlink *ioctl_handler_users;
>    };
>  
>  
> @@ -292,8 +324,15 @@ extern file_t __directory_name_split_at (int fd, const 
> char *directory_name,
>  
>  
>  /* Call D's ioctl handler, loading it from the underlying port if
> -   necessary.  Arguments are the same as ioctl handlers.  */
> -extern int _hurd_fd_call_ioctl_handler (int fd, int request, void *arg);
> +   necessary.  Arguments are the same as ioctl handlers, except for
> +   CRIT in which a new lock is returned.  */
> +extern error_t _hurd_fd_call_ioctl_handler (int fd, struct hurd_fd *d,
> +                                         void **crit, int request,
> +                                         void *arg, int *result);
> +
> +/* Ioctl handler cleanup function for non-local exits.  */
> +extern void _hurd_fd_ioctl_handler_map_cleanup (void *data, jmp_buf env,
> +                                             int val);
>  
>  
>  #endif       /* hurd/fd.h */
> diff --git a/hurd/hurd/ioctl.h b/hurd/hurd/ioctl.h
> index f932fa1..6f3f9a4 100644
> --- a/hurd/hurd/ioctl.h
> +++ b/hurd/hurd/ioctl.h
> @@ -23,11 +23,9 @@
>  #define      __need___va_list
>  #include <stdarg.h>
>  #include <bits/ioctls.h>
> +#include <hurd/fd.h>
>  
>  
> -/* Type of handler function, called like ioctl to do its entire job.  */
> -typedef int (*ioctl_handler_t) (int fd, int request, void *arg);
> -
>  /* Structure that records an ioctl handler.  */
>  struct ioctl_handler
>    {
> @@ -76,7 +74,8 @@ ioctl_handler_t _hurd_lookup_ioctl_handler (int request);
>  
>  /* A dummy handler that always fails.  */
>  
> -int _hurd_dummy_ioctl_handler (int fd, int request, void *arg);
> +error_t _hurd_dummy_ioctl_handler (int fd, struct hurd_fd *, void *crit,
> +                                int request, void *arg, int *result);
>  
>  
>  #endif       /* hurd/ioctl.h */
> diff --git a/hurd/hurdioctl.c b/hurd/hurdioctl.c
> index 0b91ee1..fb2db13 100644
> --- a/hurd/hurdioctl.c
> +++ b/hurd/hurdioctl.c
> @@ -51,21 +51,27 @@ _hurd_lookup_ioctl_handler (int request)
>  }
>  
>  /* A dummy handler that always fails.  */
> -int
> -_hurd_dummy_ioctl_handler (int fd, int request, void *arg)
> +error_t
> +_hurd_dummy_ioctl_handler (int fd, struct hurd_fd *d, void *crit,
> +                        int request, void *arg, int *result)
>  {
> -  return __hurd_fail (ENOTTY);
> +  return ENOTTY;
>  }
>  
>  #include <fcntl.h>
>  
>  /* Find out how many bytes may be read from FD without blocking.  */
>  
> -static int
> +static error_t
>  fioctl (int fd,
> +     struct hurd_fd *d,
> +     void *crit,
>       int request,
> -     int *arg)
> +     int *arg,
> +     int *result)
>  {
> +  mach_port_t port;
> +  struct hurd_userlink link;
>    error_t err;
>  
>    *(volatile int *) arg = *arg;
> @@ -73,57 +79,78 @@ fioctl (int fd,
>    switch (request)
>      {
>      default:
> -      err = ENOTTY;
> +      return ENOTTY;
> +
> +    case FIONREAD:
> +    case FIONBIO:
> +    case FIOASYNC:
> +    case FIOSETOWN:
> +    case FIOGETOWN:
>        break;
> +    }
> +
> +  port = _hurd_port_locked_get (&d->port, &link);
> +  _hurd_critical_section_unlock (crit);
>  
> +  switch (request)
> +    {
>      case FIONREAD:
>        {
>       mach_msg_type_number_t navail;
> -     err = HURD_DPORT_USE (fd, __io_readable (port, &navail));
> +     err = __io_readable (port, &navail);
>       if (!err)
>         *arg = (int) navail;
>        }
>        break;
>  
>      case FIONBIO:
> -      err = HURD_DPORT_USE (fd, (*arg ?
> -                              __io_set_some_openmodes :
> -                              __io_clear_some_openmodes)
> -                         (port, O_NONBLOCK));
> +      if (*arg)
> +     err = __io_set_some_openmodes (port, O_NONBLOCK);
> +      else
> +     err = __io_clear_some_openmodes (port, O_NONBLOCK);
>        break;
>  
>      case FIOASYNC:
> -      err = HURD_DPORT_USE (fd, (*arg ?
> -                              __io_set_some_openmodes :
> -                              __io_clear_some_openmodes)
> -                         (port, O_ASYNC));
> +      if (*arg)
> +     err = __io_set_some_openmodes (port, O_ASYNC);
> +      else
> +     err = __io_clear_some_openmodes (port, O_ASYNC);
>        break;
>  
>      case FIOSETOWN:
> -      err = HURD_DPORT_USE (fd, __io_mod_owner (port, *arg));
> +      err = __io_mod_owner (port, *arg);
>        break;
>  
>      case FIOGETOWN:
> -      err = HURD_DPORT_USE (fd, __io_get_owner (port, arg));
> +      err = __io_get_owner (port, arg);
>        break;
> +
> +    default:
> +      assert (! "unhandled ioctl number slipped through");
>      }
> +  _hurd_port_free (&d->port, &link, port);
>  
> -  return err ? __hurd_dfail (fd, err) : 0;
> +  *result = err ? __hurd_dfail (fd, err) : 0;
> +  return 0;
>  }
>  
>  _HURD_HANDLE_IOCTLS (fioctl, FIOGETOWN, FIONREAD);
>  
>  
> -static int
> +static error_t
>  fioclex (int fd,
> -      int request)
> +      struct hurd_fd *d,
> +      void *crit,
> +      int request,
> +      void *arg,
> +      int *result)
>  {
>    int flag;
>  
>    switch (request)
>      {
>      default:
> -      return __hurd_fail (ENOTTY);
> +      return ENOTTY;
>      case FIOCLEX:
>        flag = FD_CLOEXEC;
>        break;
> @@ -132,7 +159,12 @@ fioclex (int fd,
>        break;
>      }
>  
> -  return __fcntl (fd, F_SETFD, flag);
> +  d->flags = flag;
> +  __spin_unlock (&d->port.lock);
> +  _hurd_critical_section_unlock (crit);
> +
> +  *result = 0;
> +  return 0;
>  }
>  _HURD_HANDLE_IOCTLS (fioclex, FIOCLEX, FIONCLEX);
>  
> @@ -279,28 +311,53 @@ tiocsctty_internal (io_t port, io_t ctty)
>    return 0;
>  }
>  
> -static int
> +static error_t
>  tiocsctty (int fd,
> -        int request)         /* Always TIOCSCTTY.  */
> +        struct hurd_fd *d,
> +        void *crit,
> +        int request,
> +        void *arg,
> +        int *result)         /* Always TIOCSCTTY.  */
>  {
> +  struct hurd_userlink link, ctty_link;
> +  io_t port, ctty;
>    error_t err;
>  
> -  err = HURD_DPORT_USE (fd, tiocsctty_internal (port, ctty));
> -  return __hurd_fail (err);
> +  ctty = _hurd_port_get (&d->ctty, &ctty_link);
> +  port = _hurd_port_locked_get (&d->port, &link);
> +  _hurd_critical_section_unlock (crit);
> +
> +  err = tiocsctty_internal (port, ctty);
> +
> +  _hurd_port_free (&d->port, &link, port);
> +  if (ctty != MACH_PORT_NULL)
> +   _hurd_port_free (&d->ctty, &ctty_link, ctty);
> +
> +  *result = __hurd_fail (err);
> +  return 0;
>  }
>  _HURD_HANDLE_IOCTL (tiocsctty, TIOCSCTTY);
>  
>  /* Dissociate from the controlling terminal.  */
>  
> -static int
> +static error_t
>  tiocnotty (int fd,
> -        int request)         /* Always TIOCNOTTY.  */
> +        struct hurd_fd *d,
> +        void *crit,
> +        int request,
> +        void *arg,
> +        int *result)         /* Always TIOCNOTTY.  */
>  {
> +  struct hurd_userlink link;
> +  io_t dport;
>    mach_port_t fd_cttyid;
>    error_t err;
>  
> -  if (err = HURD_DPORT_USE (fd, __term_getctty (port, &fd_cttyid)))
> -    return __hurd_fail (err);
> +  dport = _hurd_port_locked_get (&d->port, &link);
> +  _hurd_critical_section_unlock (crit);
> +
> +  if (err = __term_getctty (dport, &fd_cttyid))
> +    goto out;
>  
>    if (__USEPORT (CTTYID, port != fd_cttyid))
>      err = EINVAL;
> @@ -308,11 +365,14 @@ tiocnotty (int fd,
>    __mach_port_deallocate (__mach_task_self (), fd_cttyid);
>  
>    if (err)
> -    return __hurd_fail (err);
> +    goto out;
>  
>    /* Clear our cttyid port.  */
>    install_ctty (MACH_PORT_NULL);
>  
> +out:
> +  _hurd_port_free (&d->port, &link, dport);
> +  *result = __hurd_fail (err);
>    return 0;
>  }
>  _HURD_HANDLE_IOCTL (tiocnotty, TIOCNOTTY);
> @@ -323,9 +383,12 @@ _HURD_HANDLE_IOCTL (tiocnotty, TIOCNOTTY);
>  
>  /* Fill in the buffer IFC->IFC_BUF of length IFC->IFC_LEN with a list
>     of ifr structures, one for each network interface.  */
> -static int
> -siocgifconf (int fd, int request, struct ifconf *ifc)
> +static error_t
> +siocgifconf (int fd, struct hurd_fd *d, void *crit,
> +          int request, struct ifconf *ifc, int *result)
>  {
> +  struct hurd_userlink link;
> +  io_t port;
>    error_t err;
>    size_t data_len = ifc->ifc_len;
>    char *data = ifc->ifc_buf;
> @@ -333,8 +396,10 @@ siocgifconf (int fd, int request, struct ifconf *ifc)
>    if (data_len <= 0)
>      return 0;
>  
> -  err = HURD_DPORT_USE (fd, __pfinet_siocgifconf (port, ifc->ifc_len,
> -                                               &data, &data_len));
> +  port = _hurd_port_locked_get (&d->port, &link);
> +  _hurd_critical_section_unlock (crit);
> +
> +  err = __pfinet_siocgifconf (port, ifc->ifc_len, &data, &data_len);
>    if (data_len < ifc->ifc_len)
>      ifc->ifc_len = data_len;
>    if (data != ifc->ifc_buf)
> @@ -342,6 +407,9 @@ siocgifconf (int fd, int request, struct ifconf *ifc)
>        memcpy (ifc->ifc_buf, data, ifc->ifc_len);
>        __vm_deallocate (__mach_task_self (), (vm_address_t) data, data_len);
>      }
> -  return err ? __hurd_dfail (fd, err) : 0;
> +
> +  _hurd_port_free (&d->port, &link, port);
> +  *result = err ? __hurd_dfail (fd, err) : 0;
> +  return 0;
>  }
>  _HURD_HANDLE_IOCTL (siocgifconf, SIOCGIFCONF);
> diff --git a/hurd/new-fd.c b/hurd/new-fd.c
> index 152bb35..b2b09de 100644
> --- a/hurd/new-fd.c
> +++ b/hurd/new-fd.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1994, 1997, 1999 Free Software Foundation, Inc.
> +/* Copyright (C) 1994, 1997, 1999, 2009 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -36,6 +36,11 @@ _hurd_new_fd (io_t port, io_t ctty)
>        
>        /* And the fcntl flags.  */
>        d->flags = 0;
> +
> +      /* And the ioctl handler.  */
> +      d->ioctl_handler = NULL;
> +      d->ioctl_handler_map = NULL;
> +      d->ioctl_handler_users = NULL;
>      }
>  
>    return d;
> diff --git a/hurd/port2fd.c b/hurd/port2fd.c
> index 262e6d9..4d491df 100644
> --- a/hurd/port2fd.c
> +++ b/hurd/port2fd.c
> @@ -1,4 +1,4 @@
> -/* Copyright (C) 1994, 1997, 1999, 2007 Free Software Foundation, Inc.
> +/* Copyright (C) 1994, 1997, 1999, 2007, 2009 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
>     The GNU C Library is free software; you can redistribute it and/or
> @@ -21,6 +21,7 @@
>  #include <hurd/signal.h>
>  #include <hurd/term.h>
>  #include <fcntl.h>
> +#include <dlfcn.h>
>  
>  /* Store PORT in file descriptor D, doing appropriate ctty magic.
>     FLAGS are as for `open'; only O_IGNORE_CTTY is meaningful.
> @@ -61,4 +62,12 @@ _hurd_port2fd (struct hurd_fd *d, io_t dport, int flags)
>    }
>  
>    _hurd_port_set (&d->ctty, ctty);
> +
> +  /* Clear ioctl handler associated with old port.  */
> +  if (d->ioctl_handler_map != NULL
> +    && _hurd_userlink_clear (&d->ioctl_handler_users))
> +    __libc_dlclose (d->ioctl_handler_map);
> +  d->ioctl_handler = NULL;
> +  d->ioctl_handler_map = NULL;
> +  d->ioctl_handler_users = NULL;
>  }
> diff --git a/sysdeps/mach/hurd/ioctl.c b/sysdeps/mach/hurd/ioctl.c
> index 736ad8e..bdc2fc2 100644
> --- a/sysdeps/mach/hurd/ioctl.c
> +++ b/sysdeps/mach/hurd/ioctl.c
> @@ -91,6 +91,9 @@ __ioctl (int fd, unsigned long int request, ...)
>  
>    void *arg = NULL;
>  
> +  struct hurd_fd *d;
> +  void *crit;
> +
>    error_t err;
>  
>    /* Send the RPC already packed up in MSG to IOPORT
> @@ -219,10 +222,25 @@ __ioctl (int fd, unsigned long int request, ...)
>        va_end (ap);
>      }
>  
> +  d = _hurd_fd_get (fd);
> +  if (d == NULL)
> +    return __hurd_fail (EBADF);
> +
> +  crit = _hurd_critical_section_lock ();
> +  __spin_lock (&d->port.lock);
> +
> +  if (d->port.port == MACH_PORT_NULL)
> +    {
> +      __spin_unlock (&d->port.lock);
> +      _hurd_critical_section_unlock (crit);
> +      return __hurd_fail (EBADF);
> +    }
> +
>    {
>      int save = errno;
> -    int result = _hurd_fd_call_ioctl_handler (fd, request, arg);
> -    if (result != -1 || errno != ENOTTY)
> +    int result;
> +    err = _hurd_fd_call_ioctl_handler (fd, d, &crit, request, arg, &result);
> +    if (!err)
>        return result;
>  
>      /* The handler doesn't grok this one.  Try linked handlers.  */
> @@ -230,14 +248,14 @@ __ioctl (int fd, unsigned long int request, ...)
>    }
>  
>    {
> -    /* Check for a registered handler for REQUEST.  */
>      ioctl_handler_t handler = _hurd_lookup_ioctl_handler (request);
>      if (handler)
>        {
>       /* This handler groks REQUEST.  Se lo puntamonos.  */
>       int save = errno;
> -     int result = (*handler) (fd, request, arg);
> -     if (result != -1 || errno != ENOTTY)
> +     int result;
> +     err = (*handler) (fd, d, crit, request, arg, &result);
> +     if (!err)
>         return result;
>  
>       /* The handler doesn't really grok this one.
> @@ -280,7 +298,20 @@ __ioctl (int fd, unsigned long int request, ...)
>    /* Marshal the arguments into the request message and make the RPC.
>       This wrapper function handles EBACKGROUND returns, turning them
>       into either SIGTTOU or EIO.  */
> -  err = HURD_DPORT_USE (fd, _hurd_ctty_output (port, ctty, send_rpc));
> +  {
> +    io_t port, ctty;
> +    struct hurd_userlink ulink, ctty_ulink;
> +
> +    ctty = _hurd_port_get (&d->ctty, &ctty_ulink);
> +    port = _hurd_port_locked_get (&d->port, &ulink);
> +    _hurd_critical_section_unlock (crit);
> +
> +    err = _hurd_ctty_output (port, ctty, send_rpc);
> +
> +    _hurd_port_free (&d->port, &ulink, port);
> +    if (ctty != MACH_PORT_NULL)
> +      _hurd_port_free (&d->ctty, &ctty_ulink, ctty);
> +  }
>  
>  #ifdef MACH_MSG_TYPE_BIT
>    t = (mach_msg_type_t *) msg.data;

I can't really comment on this locking stuff -- I'm not familiar with
libc code, nor experienced with locking in general.

BTW, could you split this into two patches: one for the locking changes;
and one for the actual persistent handler changes?... Should be much
easier to follow.

-antrik-