devscgroup: relax task to dev_cgroup conversion
PATCH 2/2 devscgroup: relax task to dev_cgroup conversion
Two functions, that need to get a device_cgroup from a task (they are devcgroup_inode_permission and devcgroup_inode_mknod) make it in a strange way:
They get a css_set from task, then a subsys_state from css_set, then a cgroup from the state and then a subsys_state again from the cgroup. Besides, the devices_subsys_id is read from memory, whilst there's a enum-ed constant for it.
Optimize this part a bit:
1. Get the subsys_stats form the task and be done - no 2 extra dereferences,
2. Use the device_subsys_id constant, not the value from memory (i.e. one less dereference).
Found while preparing 2.6.26 OpenVZ port.
--- security/device_cgroup.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)
security/device_cgroup.c +4 -6
@@ -506,7 +506,6 @@ struct cgroup_subsys devices_subsys = {int devcgroup_inode_permission (struct inode *inode,int mask) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh;
@@ -515,8 +514,8 @@ int devcgroup_inode_permission(struct inode *inode, int mask)return 0 ;if (!S_ISBLK (inode->i_mode) && !S_ISCHR (inode->i_mode))return 0 ; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ;
@@ -547,12 +546,11 @@ acc_check:int devcgroup_inode_mknod (int mode,dev_t dev) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ;
Signature · 1 line
1.5.3.4
Quoted 15 lines · Pavel Emelyanov
Two functions, that need to get a
device_cgroupfrom a task (they aredevcgroup_inode_permissionanddevcgroup_inode_mknod) make it in a strange way:They get a
css_setfrom task, then asubsys_statefromcss_set, then a cgroup from the state and then asubsys_stateagain from the cgroup. Besides, thedevices_subsys_idis read from memory, whilst there's a enum-ed constant for it.Optimize this part a bit:
1. Get thesubsys_statsform the task and be done - no 2 extra dereferences,
2. Use thedevice_subsys_idconstant, not the value from memory (i.e. one less dereference).
Quoted 3 lines
Found while preparing 2.6.26 OpenVZ port.
Signed-off-by Pavel Emelyanov <[email protected]>
Yes, that's more how the css_set pointers are meant to be used. Maybe improve it further by extracting a task_to_devcgroup() inline function?
Paul
Quoted 47 lines
--- security/device_cgroup.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)security/device_cgroup.c +4
-7@@ -506,7 +506,6 @@ struct cgroup_subsys devices_subsys = {int devcgroup_inode_permission (struct inode *inode,int mask) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh;@@ -515,8 +514,8 @@ int devcgroup_inode_permission(struct inode *inode, int mask)return 0 ;if (!S_ISBLK (inode->i_mode) && !S_ISCHR (inode->i_mode))return 0 ; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ;@@ -547,12 +546,11 @@ acc_check:int devcgroup_inode_mknod (int mode,dev_t dev) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ; --1.5.3.4
Quoted 18 lines · Paul Menage
Quoted 17 lines · Pavel Emelyanov
Two functions, that need to get a
device_cgroupfrom a task (they aredevcgroup_inode_permissionanddevcgroup_inode_mknod) make it in a strange way:They get a
css_setfrom task, then asubsys_statefromcss_set, then a cgroup from the state and then asubsys_stateagain from the cgroup. Besides, thedevices_subsys_idis read from memory, whilst there's a enum-ed constant for it.Optimize this part a bit:
1. Get thesubsys_statsform the task and be done - no 2 extra dereferences,
2. Use thedevice_subsys_idconstant, not the value from memory (i.e. one less dereference).Found while preparing 2.6.26 OpenVZ port.
Quoted 7 lines
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>Acked-by Paul Menage <[email protected]>Yes, that's more how the
css_setpointers are meant to be used. Maybe improve it further by extracting atask_to_devcgroup()inline function?
Well... Maybe. I'll make a 3rd patch a bit later.
I'm now testing RCU-protected whitelists.
Quoted 49 lines
Paul
Quoted 46 lines
--- security/device_cgroup.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-)security/device_cgroup.c +4
-7@@ -506,7 +506,6 @@ struct cgroup_subsys devices_subsys = {int devcgroup_inode_permission (struct inode *inode,int mask) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh;@@ -515,8 +514,8 @@ int devcgroup_inode_permission(struct inode *inode, int mask)return 0 ;if (!S_ISBLK (inode->i_mode) && !S_ISCHR (inode->i_mode))return 0 ; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ;@@ -547,12 +546,11 @@ acc_check:int devcgroup_inode_mknod (int mode,dev_t dev) { -struct cgroup *cgroup;struct dev_cgroup *dev_cgroup;struct dev_whitelist_item *wh; - cgroup =task_cgroup (current, devices_subsys.subsys_id); - dev_cgroup =cgroup_to_devcgroup (cgroup); + dev_cgroup =css_to_devcgroup (task_subsys_state (current, + devices_subsys_id));if (!dev_cgroup)return 0 ; --1.5.3.4