Skip to contentSkip to conversation

cpuset: remove an unncessary forward declaration

PATCH Applied Li Zefan 2 participants 8 messages Aug 13, 2013 – Aug 16, 2013 cgroup

lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 cpuset: remove an unncessary forward declaration

Signed-off-by Li Zefan <[email protected]>
---
 kernel/cpuset.c | 3 ---
 1 file changed, 3 deletions(-)
kernel/cpuset.c +0 -3
index bf69717..9d8d637 100644
@@ -68,9 +68,6 @@
  */
 int number_of_cpusets __read_mostly;
 
-/* Forward declare cgroup structures */
-struct cgroup_subsys cpuset_subsys;
-
 /* See "Frequency meter" comments, below. */
 
 struct fmeter {
Signature · 1 line
1.8.0.2
author
#1 lore raw

PATCH 2/2 cpuset: remove redundant checks in file write functions

Now cgroup core gets a reference to the css when a cgroup file is opened(), and the reference is dropped at file release. so it's guaranteed the cpuset is online during the write function.

Signed-off-by Li Zefan <[email protected]>
---
 kernel/cpuset.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)
kernel/cpuset.c +2 -8
index 9d8d637..8c4b3c4 100644
@@ -1589,8 +1589,6 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 	int retval = -ENODEV;
 
 	mutex_lock(&cpuset_mutex);
-	if (!is_cpuset_online(cs))
-		goto out_unlock;
 
 	switch (type) {
 	case FILE_CPU_EXCLUSIVE:

@@ -1624,7 +1622,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = -EINVAL;
 		break;
 	}
-out_unlock:
+
 	mutex_unlock(&cpuset_mutex);
 	return retval;
 }

@@ -1637,8 +1635,6 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
 	int retval = -ENODEV;
 
 	mutex_lock(&cpuset_mutex);
-	if (!is_cpuset_online(cs))
-		goto out_unlock;
 
 	switch (type) {
 	case FILE_SCHED_RELAX_DOMAIN_LEVEL:

@@ -1648,7 +1644,7 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = -EINVAL;
 		break;
 	}
-out_unlock:
+
 	mutex_unlock(&cpuset_mutex);
 	return retval;
 }

@@ -1677,8 +1673,6 @@ static int cpuset_write_resmask(struct cgroup_subsys_state *css,
 	flush_work(&cpuset_hotplug_work);
 
 	mutex_lock(&cpuset_mutex);
-	if (!is_cpuset_online(cs))
-		goto out_unlock;
 
 	trialcs = alloc_trial_cpuset(cs);
 	if (!trialcs) {
Signature · 1 line
1.8.0.2
author
#2 lore raw

Re: [PATCH 2/2] cpuset: remove redundant checks in file write functions

Quoted 9 lines · Li Zefan

Now cgroup core gets a reference to the css when a cgroup file is opened(), and the reference is dropped at file release. so it's guaranteed the cpuset is online during the write function.

Signed-off-by Li Zefan <[email protected]>
---
 kernel/cpuset.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Please don't apply this patch. I'll send a new one when the bug fix I just sent is accepted.

maintainer
#3 lore raw

Re: [PATCH 2/2] cpuset: remove redundant checks in file write functions

In reply to Li Zefan
Quoted 3 lines · Li Zefan

Now cgroup core gets a reference to the css when a cgroup file is opened(), and the reference is dropped at file release. so it's guaranteed the cpuset is online during the write function.

Hmmm... it doesn't really guarantee that as css's can be offlined with residual css refcnts, os the css may well be offlined by the time it reaches the rw functions. What's guaranteed is that their refcnts wouldn't be zero. Eventually we need to implement proper sever semantics (probably by replacing the custom fs implementation with sysfs) but right now controllers still need to deal with offline css's.

Thanks.

Signature · 1 line
tejun
author
#4 lore raw

Re: [PATCH 2/2] cpuset: remove redundant checks in file write functions

Quoted 9 lines · Tejun Heo
Quoted 3 lines · Li Zefan

Now cgroup core gets a reference to the css when a cgroup file is opened(), and the reference is dropped at file release. so it's guaranteed the cpuset is online during the write function.

Hmmm... it doesn't really guarantee that as css's can be offlined with residual css refcnts, os the css may well be offlined by the time it reaches the rw functions. What's guaranteed is that their refcnts wouldn't be zero.

Oh, right.

But most controllers don't check this in those read/write functions. It shoudn't do any harm not checking online/offline status.

Quoted 7 lines

Eventually we need to implement proper sever
semantics (probably by replacing the custom fs implementation with sysfs) but right now controllers still need to deal with offline css's.

Thanks.

maintainer
#5 lore raw

Re: [PATCH 2/2] cpuset: remove redundant checks in file write functions

Hello, Li.

Quoted 2 lines · Li Zefan

But most controllers don't check this in those read/write functions. It shoudn't do any harm not checking online/offline status.

It depends on the specific controller. For controllers which make clear distinction between online and offline states for, say, hierarchical config propagation, it could lead to buggy behavior if the function body afterwards assume that the current node is online. I don't know whether this is the case for the functions converted here but even if so the patch needs to update the description.

Thanks.

Signature · 1 line
tejun
author
#6 lore raw

Re: [PATCH 2/2] cpuset: remove redundant checks in file write functions

Quoted 10 lines · Tejun Heo

Hello, Li.

Quoted 2 lines · Li Zefan

But most controllers don't check this in those read/write functions. It shoudn't do any harm not checking online/offline status.

It depends on the specific controller. For controllers which make clear distinction between online and offline states for, say, hierarchical config propagation, it could lead to buggy behavior if the function body afterwards assume that the current node is online.

In those cases, there must be some comments, otherwise it would be confusing if and when one needs to check the online/offline states.

Quoted 3 lines

I don't know whether this is the case for the functions converted here but even if so the patch needs to update the description.

I'll check for sure.