* [PATCH] net: net_cls: remove a NULL check for css_cls_state
@ 2018-04-19 4:59 Li RongQing
2018-04-20 14:37 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Li RongQing @ 2018-04-19 4:59 UTC (permalink / raw)
To: netdev
The input of css_cls_state() is impossible to NULL except
cgrp_css_online, so simplify it
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/core/netclassid_cgroup.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/core/netclassid_cgroup.c b/net/core/netclassid_cgroup.c
index 5e4f04004a49..ee087cf793c2 100644
--- a/net/core/netclassid_cgroup.c
+++ b/net/core/netclassid_cgroup.c
@@ -19,7 +19,7 @@
static inline struct cgroup_cls_state *css_cls_state(struct cgroup_subsys_state *css)
{
- return css ? container_of(css, struct cgroup_cls_state, css) : NULL;
+ return container_of(css, struct cgroup_cls_state, css);
}
struct cgroup_cls_state *task_cls_state(struct task_struct *p)
@@ -44,10 +44,9 @@ cgrp_css_alloc(struct cgroup_subsys_state *parent_css)
static int cgrp_css_online(struct cgroup_subsys_state *css)
{
struct cgroup_cls_state *cs = css_cls_state(css);
- struct cgroup_cls_state *parent = css_cls_state(css->parent);
- if (parent)
- cs->classid = parent->classid;
+ if (css->parent)
+ cs->classid = css_cls_state(css->parent)->classid;
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: net_cls: remove a NULL check for css_cls_state
2018-04-19 4:59 [PATCH] net: net_cls: remove a NULL check for css_cls_state Li RongQing
@ 2018-04-20 14:37 ` David Miller
2018-04-25 8:35 ` Li RongQing
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2018-04-20 14:37 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
From: Li RongQing <lirongqing@baidu.com>
Date: Thu, 19 Apr 2018 12:59:21 +0800
> The input of css_cls_state() is impossible to NULL except
> cgrp_css_online, so simplify it
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
I don't view this as an improvement. Just let the helper always check
NULL and that way there are less situations to audit.
And it's not like this is a critical fast path either.
I'm not applying this, sorry.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: net_cls: remove a NULL check for css_cls_state
2018-04-20 14:37 ` David Miller
@ 2018-04-25 8:35 ` Li RongQing
2018-04-25 15:00 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Li RongQing @ 2018-04-25 8:35 UTC (permalink / raw)
To: David Miller; +Cc: lirongqing, netdev
On 4/20/18, David Miller <davem@davemloft.net> wrote:
> From: Li RongQing <lirongqing@baidu.com>
> Date: Thu, 19 Apr 2018 12:59:21 +0800
>
>> The input of css_cls_state() is impossible to NULL except
>> cgrp_css_online, so simplify it
>>
>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>
> I don't view this as an improvement. Just let the helper always check
> NULL and that way there are less situations to audit.
>
css_cls_state maybe return NULL, but nearly no places check the return
value with NULL, this seems unreadable.
net/core/netclassid_cgroup.c:27: return
css_cls_state(task_css_check(p, net_cls_cgrp_id,
net/core/netclassid_cgroup.c:46: struct cgroup_cls_state *cs =
css_cls_state(css);
net/core/netclassid_cgroup.c:47: struct cgroup_cls_state
*parent = css_cls_state(css->parent);
net/core/netclassid_cgroup.c:57: kfree(css_cls_state(css));
net/core/netclassid_cgroup.c:82: (void
*)(unsigned long)css_cls_state(css)->classid);
net/core/netclassid_cgroup.c:89: return css_cls_state(css)->classid;
net/core/netclassid_cgroup.c:95: struct cgroup_cls_state *cs =
css_cls_state(css);
> And it's not like this is a critical fast path either.
>
I see css_cls_state will be called when send packet if
CONFIG_NET_CLS_ACT and CONFIG_NET_EGRESS enabled, the calling stack is
like below:
css_cls_state
task_cls_state
task_get_classid
cls_cgroup_classify
tcf_classify
sch_handle_egress
__dev_queue_xmit
CONFIG_NET_CLS_ACT
CONFIG_NET_EGRESS
-RongQing
> I'm not applying this, sorry.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: net_cls: remove a NULL check for css_cls_state
2018-04-25 8:35 ` Li RongQing
@ 2018-04-25 15:00 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-04-25 15:00 UTC (permalink / raw)
To: roy.qing.li; +Cc: lirongqing, netdev
From: Li RongQing <roy.qing.li@gmail.com>
Date: Wed, 25 Apr 2018 16:35:02 +0800
> On 4/20/18, David Miller <davem@davemloft.net> wrote:
>> From: Li RongQing <lirongqing@baidu.com>
>> Date: Thu, 19 Apr 2018 12:59:21 +0800
>>
>>> The input of css_cls_state() is impossible to NULL except
>>> cgrp_css_online, so simplify it
>>>
>>> Signed-off-by: Li RongQing <lirongqing@baidu.com>
>>
>> I don't view this as an improvement. Just let the helper always check
>> NULL and that way there are less situations to audit.
>>
> css_cls_state maybe return NULL, but nearly no places check the return
> value with NULL, this seems unreadable.
I saw this email of your's the first few times, you don't need to post
it again.
I still disagree with your analysis, and I do not see your change as
an overall improvement.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-25 15:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-19 4:59 [PATCH] net: net_cls: remove a NULL check for css_cls_state Li RongQing
2018-04-20 14:37 ` David Miller
2018-04-25 8:35 ` Li RongQing
2018-04-25 15:00 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.