On 12.09.2022 10:36, Julien Grall wrote: > On 12/09/2022 09:33, Juergen Gross wrote: >> On 12.09.22 10:31, Jan Beulich wrote: >>> On 12.09.2022 10:23, Juergen Gross wrote: >>>> On 12.09.22 10:19, Jan Beulich wrote: >>>>> On 12.09.2022 07:53, Juergen Gross wrote: >>>>>> Add a helper domid_to_domain() returning the struct domain pointer for >>>>>> a domain give by its domid and which is known not being able to be >>>>>> released (its reference count isn't incremented and no >>>>>> rcu_lock_domain() >>>>>> is called for it). >>>>>> >>>>>> In order to simplify coding add an internal helper for doing the >>>>>> lookup >>>>>> and call that from the new function and similar functions. >>>>>> >>>>>> Signed-off-by: Juergen Gross <jgr...@suse.com> >>>>> >>>>> I don't see an issue with adding such a helper (responding to your >>>>> concern >>>>> in the cover letter), but I think the constraints need to be empahsized >>>>> more: We already have get_knownalive_domain() and >>>>> get_domain_by_id(), so >>>>> how about naming the new helper get_knownalive_domain_by_id()? And >>>>> then ... >>>> >>>> I explicitly didn't name it "get_...", as those helpers all increment >>>> the >>>> reference count of the domain. And this is NOT done by the new helper. >>> >>> Hmm, agreed. But domid_to_domain() isn't expressing the "known alive" >>> aspect, >>> yet that's relevant to see when reviewing new uses of the function. >>> Such uses >>> aren't likely to make the reviewer go look at the function declaration >>> when >>> the function name is pretty "innocent". >> >> Okay, what about domid_to_knownalive_domain()? >> >> Or knownalive_domain_from_domid()? > > FWIW, I am fine with either. However, please don't replace 'to' with '2' > if you need a internal helper. Just suffixed with 'locked' to make clear > this is a version where the caller should take the lock.
Hmm - personally I dislike "_locked" suffixes on functions. If the "internal helper" aspect is to be made more explicit, then perhaps by way of prefixing a single underscore? Jan
