On Tue, Aug 19, 2014 at 09:15:35AM +0200, Daniel Mack wrote: > Hi Djalal, Thanks for applying the others.
> 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 <[email protected]> > > --- > > 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. Hmm with your commit 7bde48f293f5, I guess we have to add a generic struct like that, or split into multiple structs, the aim would be to share the struct and operations that perform the: Translate and install fields into the receiver's slice when requesting the connection info or when the receiver reads the message ? > [...] > > > /** > > + * 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. Yes, I agree that the code would not be easy to read, and I failed to come up with a better solution, perhaps it needs more thoughts! Well, yes without numbers the optimization for uid/gid perhaps is not needed, but for the auxgroup perhaps ? we can allocate and get the auxgrps one time, then just memdup the data into the queue->auxgrps[] ? Just trying to figure out the best solution... BTW Daniel, sorry for this stupid question: Can't the KDBUS_ATTACH_AUXGROUPS be part of the KDBUS_ATTACH_CREDS item? Will be easy to parse it in the same item with uid/gid... -- Djalal Harouni http://opendz.org _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
