Re: active swapping using rumpdisk

[Samuel Thibault <sthibault@debian.org>]

Michael Kelly, le mar. 19 août 2025 12:48:55 +0100, a ecrit:
> I specified different constants than the IKOT_ ones for the RPC for 'ktype'
> (see port.h). This avoids the issue of how to export the IKOT_ defines to
> the user interface and in any case makes it clear which IKOT types are
> appropriate.

That should be fine indeed. I'd however say we'd probably want to make
MACH_PORT_KTYPE_USER_DEVICE 28 like in the kernel macro.

> diff -ur gnumach.orig/ipc/mach_port.c gnumach/ipc/mach_port.c
> --- gnumach.orig/ipc/mach_port.c	2025-07-31 20:34:14.844151887 +0100
> +++ gnumach/ipc/mach_port.c	2025-08-19 05:40:55.423218382 +0100
> @@ -1567,6 +1567,44 @@
>  	return KERN_SUCCESS;
>  }
>  
> +kern_return_t
> +mach_port_set_ktype(
> +        host_t host_priv,
> +        ipc_space_t space,
> +        mach_port_name_t name,
> +        mach_port_right_t right,
> +        unsigned ktype)
> +{
> +	ipc_port_t port;
> +	kern_return_t kr;
> +
> +	if (host_priv == HOST_NULL)
> +		return KERN_INVALID_HOST;
> +
> +	if (space == IS_NULL)
> +		return KERN_INVALID_TASK;
> +
> +	if (ktype != MACH_PORT_KTYPE_NONE
> +	    && ktype != MACH_PORT_KTYPE_USER_DEVICE)
> +	  return KERN_INVALID_ARGUMENT;

Please mind the non-coherent indent of return.

> +	kr = ipc_object_translate(space, name, right, (ipc_object_t *)&port);
> +	if (kr != KERN_SUCCESS)
> +		return kr;
> +
> +	if (ip_kotype(port) != IKOT_NONE && ip_kotype(port) != IKOT_USER_DEVICE)

We'd need to unlock the port here, don't we?

> +	  return KERN_INVALID_ARGUMENT;
> +
> +	/* port is locked and active */
> +	ipc_kobject_set(port, IKO_NULL,
> +			ktype == MACH_PORT_KTYPE_NONE
> +			? IKOT_NONE
> +			: IKOT_USER_DEVICE);
> +	ip_unlock(port);
> +
> +	return KERN_SUCCESS;
> +}
> +
>  #if	MACH_KDB
>  
>  void


> diff -ur hurd.orig/rumpdisk/block-rump.c hurd/rumpdisk/block-rump.c
> --- hurd.orig/rumpdisk/block-rump.c	2025-07-31 20:36:06.014917781 +0100
> +++ hurd/rumpdisk/block-rump.c	2025-08-19 08:20:48.477079651 +0100
> @@ -27,6 +27,7 @@
>  
>  #include <mach.h>
>  #include <mach/gnumach.h>
> +#include <mach/port.h>
>  #include <hurd.h>
>  #include <hurd/ports.h>
>  #include <device/device.h>
> @@ -327,6 +328,25 @@
>        rump_sys_close (fd);
>        pthread_rwlock_unlock (&rumpdisk_rwlock);
>        return err;
> +    }
> +
> +  /* Configure the receive port as a USER_DEVICE so that IPC messages
> +     destined for rumpdisk will use page lists rather than page map
> +     entries. This strategy prevents pages that are referenced in the
> +     message body from being swapped out until the message has been
> +     processed.
> +  */
> +  err = mach_port_set_ktype (master_host,
> +			     mach_task_self (),
> +			     bd->port.port_right,
> +			     MACH_PORT_RIGHT_RECEIVE,
> +			     MACH_PORT_KTYPE_USER_DEVICE);
> +  if (err != 0)
> +    {
> +      mach_print ("Failed to set receive port as USER_DEVICE\n");
> +      rump_sys_close (fd);
> +      pthread_rwlock_unlock (&rumpdisk_rwlock);
> +      return err;
>      }
>  
>    bd->taken = 1;

We don't want to make this fatal, because that would immediately break a
box that upgrades its hurd package but not its gnumach package. Better
just print a fat warning that swapping might hang.

Also, better print it on stderr and fflush() it. The other mach_print
are just debugging prints that shouldn't ever happen.

Samuel