diff mbox series

cgroup: remove unneeded return value of cgroup_rm_cftypes_locked()

Message ID 20230701073856.2095425-1-linmiaohe@huawei.com
State New
Headers
Series cgroup: remove unneeded return value of cgroup_rm_cftypes_locked() |

Commit Message

Miaohe Lin July 1, 2023, 7:38 a.m. UTC 
  The return value of cgroup_rm_cftypes_locked() is always 0. So remove
it to simplify the code. No functional change intended.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 kernel/cgroup/cgroup.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Michal Koutný July 10, 2023, 3:46 p.m. UTC | #1 
Hi.

On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin <linmiaohe@huawei.com> wrote:
> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
> it to simplify the code. No functional change intended.

I'd add a comment that it builds upon cgroup_addrm_files()'s:
> For removals, this function never fails.

> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  kernel/cgroup/cgroup.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)

Reviewed-by: Michal Koutný <mkoutny@suse.com>
Tejun Heo July 10, 2023, 7:40 p.m. UTC | #2 
On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin wrote:
> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
> it to simplify the code. No functional change intended.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>

Applied to cgroup/for-6.6. Please feel free to follow up with the comment
addition Michal suggested.

Thanks.
Miaohe Lin July 11, 2023, 2:58 a.m. UTC | #3 
On 2023/7/10 23:46, Michal Koutný wrote:
> Hi.
> 
> On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin <linmiaohe@huawei.com> wrote:
>> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
>> it to simplify the code. No functional change intended.
> 
> I'd add a comment that it builds upon cgroup_addrm_files()'s:

Do you mean something like below:

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index b0d98542eea2..2c02f319a7d4 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4320,6 +4320,7 @@ static int cgroup_init_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
        return ret;
 }

+/* It builds upon cgroup_addrm_files()'s. */
 static void cgroup_rm_cftypes_locked(struct cftype *cfts)
 {
        lockdep_assert_held(&cgroup_mutex);


>> For removals, this function never fails.
> 
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
>>  kernel/cgroup/cgroup.c | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Michal Koutný <mkoutny@suse.com>

Thanks for review and suggestion.
Miaohe Lin July 11, 2023, 3 a.m. UTC | #4 
On 2023/7/11 3:40, Tejun Heo wrote:
> On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin wrote:
>> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
>> it to simplify the code. No functional change intended.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> 
> Applied to cgroup/for-6.6. Please feel free to follow up with the comment
> addition Michal suggested.

Should I send a v2 patch or a separate patch? Both is fine to me.

Thanks for your work.
Michal Koutný July 11, 2023, 11:21 a.m. UTC | #5 
On Tue, Jul 11, 2023 at 10:58:48AM +0800, Miaohe Lin <linmiaohe@huawei.com> wrote:
> +/* It builds upon cgroup_addrm_files()'s. */
>  static void cgroup_rm_cftypes_locked(struct cftype *cfts)
>  {
>         lockdep_assert_held(&cgroup_mutex);
> 

I meant adding the reasoning to the commit message -- swallowing errors
on removal is fine because cgroup_addrm_files() won't fail at removal.
It's a minor remark for the future readers ;-)

Michal
Tejun Heo July 11, 2023, 9:40 p.m. UTC | #6 
On Tue, Jul 11, 2023 at 11:00:58AM +0800, Miaohe Lin wrote:
> On 2023/7/11 3:40, Tejun Heo wrote:
> > On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin wrote:
> >> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
> >> it to simplify the code. No functional change intended.
> >>
> >> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > 
> > Applied to cgroup/for-6.6. Please feel free to follow up with the comment
> > addition Michal suggested.
> 
> Should I send a v2 patch or a separate patch? Both is fine to me.

Please send a separate patch.

Thanks.
Miaohe Lin July 12, 2023, 2:02 a.m. UTC | #7 
On 2023/7/12 5:40, Tejun Heo wrote:
> On Tue, Jul 11, 2023 at 11:00:58AM +0800, Miaohe Lin wrote:
>> On 2023/7/11 3:40, Tejun Heo wrote:
>>> On Sat, Jul 01, 2023 at 03:38:56PM +0800, Miaohe Lin wrote:
>>>> The return value of cgroup_rm_cftypes_locked() is always 0. So remove
>>>> it to simplify the code. No functional change intended.
>>>>
>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>>
>>> Applied to cgroup/for-6.6. Please feel free to follow up with the comment
>>> addition Michal suggested.
>>
>> Should I send a v2 patch or a separate patch? Both is fine to me.
> 
> Please send a separate patch.

I see. But since Michal is meant adding the reasoning to the commit message, it seems
a v2 patch is required. Or could you help modify the commit message? It should looks
like:

"
The return value of cgroup_rm_cftypes_locked() is always 0 and swallowing
errors on removal is fine because cgroup_addrm_files() won't fail at
removal. So remove return value to simplify the code. No functional
change intended.
"

Thanks a lot.
diff mbox series

Patch

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index bfe3cd8ccf36..b0d98542eea2 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4320,14 +4320,13 @@  static int cgroup_init_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 	return ret;
 }
 
-static int cgroup_rm_cftypes_locked(struct cftype *cfts)
+static void cgroup_rm_cftypes_locked(struct cftype *cfts)
 {
 	lockdep_assert_held(&cgroup_mutex);
 
 	list_del(&cfts->node);
 	cgroup_apply_cftypes(cfts, false);
 	cgroup_exit_cftypes(cfts);
-	return 0;
 }
 
 /**
@@ -4343,8 +4342,6 @@  static int cgroup_rm_cftypes_locked(struct cftype *cfts)
  */
 int cgroup_rm_cftypes(struct cftype *cfts)
 {
-	int ret;
-
 	if (!cfts || cfts[0].name[0] == '\0')
 		return 0;
 
@@ -4352,9 +4349,9 @@  int cgroup_rm_cftypes(struct cftype *cfts)
 		return -ENOENT;
 
 	cgroup_lock();
-	ret = cgroup_rm_cftypes_locked(cfts);
+	cgroup_rm_cftypes_locked(cfts);
 	cgroup_unlock();
-	return ret;
+	return 0;
 }
 
 /**