[systemd-devel] [PATCH 3/3] kdbus: get some creds during meta append for optimization

[Daniel Mack]

Hi Djalal,

On 08/19/2014 03:43 AM, Djalal Harouni wrote:
> Some creds can be gathered during kdbus_meta_append() instead of
> kdbus_conn_queue_alloc() where they will be gathered for all the
> receivers and saved into each receivers queue before installing
> on the slices.
> 
> By moving to kdbus_meta_append() it permits to do it only one time
> for all the receivers, and we reduce some latency in
> kdbus_conn_queue_alloc().
> 
> Another point is that all the receivers will get the same uid/gid even
> if the current sender changes its creds while we are still queueing for
> all receivers.
> 
> It seems that we can do the same with auxgroups, but this patch does
> not implement it.
> 
> Patch tested with the test-kdbus-metadata-ns
> 
> Signed-off-by: Djalal Harouni <tixxdz at opendz.org>
> ---
>  connection.c | 56 ++++++++++++++++++++++++++++++++++++--------------------
>  metadata.c   |  6 +++++-
>  metadata.h   | 26 ++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 21 deletions(-)
> 
> diff --git a/connection.c b/connection.c
> index 9250dab..49c5045 100644
> --- a/connection.c
> +++ b/connection.c
> @@ -42,6 +42,24 @@
>  struct kdbus_conn_reply;
>  
>  /**
> + * struct kdbus_queue_creds - internal creds data for receiver's
> + *			      queue
> + * @uid:		The UID to patch into the final message
> + * @gid:		The GID to patch into the final message
> + * @pid:		The PID to patch into the final message
> + * @tid:		The TID to patch into the final message
> + *
> + * Creds that must be translated into the receiver's namespaces
> + * when the message is installed into its slice.
> + */
> +struct kdbus_queue_creds {
> +	kuid_t uid;
> +	kgid_t gid;
> +	struct pid *pid;
> +	struct pid *tid;
> +};

Hmm, I'm not convinced this buys us anything really. After all, that
struct has a single user only, and factoring out these fields doesn't
necessarily lead to more readability.

[...]

>  /**
> + * struct kdbus_meta_creds - internal creds data
> + * @uid:                The UID of the sender
> + * @gid:                The GID of the sender
> + *
> + * Creds used for optimizations. We need to gather the info during
> + * kdbus_meta_append() before pushing into the receiver's queue,
> + * this way we do it only one time for all the receivers.

I'm not strictly against it, but I wonder if this optimization is really
worth it. Do you think it's that expensive to access these fields in
'current'? Because this change certainly doesn't make the code easier to
understand, so I'm hesitant about this change.


Thanks,
Daniel