On Thursday 12 February 2009 02:50:49 pm Eric Paris wrote:
> we are often needlessly jumping through hoops when it comes to avd
> entries in avc_has_perm_noaudit and we have extra initialization and memcpy
> which are just wasting performance. Try to clean the function up a bit.
>
> This patch resulted in a 13% drop in time spent in avc_has_perm_noaudit in
> my oprofile sampling of a tbench benchmark.
>
> Signed-off-by: Eric Paris <eparis@xxxxxxxxxx>
Looks fine to me, some more not-your-fault comments below.
Reviewed-by: Paul Moore <paul.moore@xxxxxx>
> static inline struct avc_node *avc_search_node(u32 ssid, u32 tsid, u16
> tclass) @@ -440,31 +440,31 @@ static int avc_latest_notif_update(int seqno,
> int is_insert) * @ssid: source security identifier
> * @tsid: target security identifier
> * @tclass: target security class
> - * @ae: AVC entry
> + * @avd: resulting av decision
> *
> * Insert an AVC entry for the SID pair
> * (@ssid, @tsid) and class @tclass.
> * The access vectors and the sequence number are
> * normally provided by the security server in
> * response to a security_compute_av() call. If the
> - * sequence number @ae->avd.seqno is not less than the latest
> + * sequence number @avd->seqno is not less than the latest
> * revocation notification, then the function copies
> * the access vectors into a cache entry, returns
> * avc_node inserted. Otherwise, this function returns NULL.
> */
> -static struct avc_node *avc_insert(u32 ssid, u32 tsid, u16 tclass, struct
> avc_entry *ae) +static struct avc_node *avc_insert(u32 ssid, u32 tsid, u16
> tclass, struct av_decision *avd) {
> struct avc_node *pos, *node = NULL;
> int hvalue;
> unsigned long flag;
>
> - if (avc_latest_notif_update(ae->avd.seqno, 1))
> + if (avc_latest_notif_update(avd->seqno, 1))
> goto out;
Not your fault but why not change the "goto out;" to "return NULL;" and get
rid of the "out" label.
> node = avc_alloc_node();
> if (node) {
Hmm, while you're at it how about converting to ...
node = avc_alloc_node();
if (node == NULL)
return NULL;
... this seems to be a better fit with the "best practices" and it makes the
code a bit more tidy (especially the "found" label, you could probably rename
that to "out" ;) ).
> hvalue = avc_hash(ssid, tsid, tclass);
> - avc_node_populate(node, ssid, tsid, tclass, ae);
> + avc_node_populate(node, ssid, tsid, tclass, avd);
>
> spin_lock_irqsave(&avc_cache.slots_lock[hvalue], flag);
> list_for_each_entry(pos, &avc_cache.slots[hvalue], list) {
--
paul moore
linux @ hp
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.
