From: Madhuparna Bhowmik <[email protected]>
This patch fixes the following sparse error:
kernel/exit.c:627:25: error: incompatible types in comparison expression
And the following warning:
kernel/exit.c:626:40: warning: incorrect type in assignment
Signed-off-by: Madhuparna Bhowmik <[email protected]>
---
kernel/exit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index bcbd59888e67..daf827a4aa25 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -623,8 +623,8 @@ static void forget_original_parent(struct task_struct *father,
reaper = find_new_reaper(father, reaper);
list_for_each_entry(p, &father->children, sibling) {
for_each_thread(p, t) {
- t->real_parent = reaper;
- BUG_ON((!t->ptrace) != (t->parent == father));
+ RCU_INIT_POINTER(t->real_parent, reaper);
+ BUG_ON((!t->ptrace) != (rcu_access_pointer(t->parent) == father));
if (likely(!t->ptrace))
t->parent = t->real_parent;
if (t->pdeath_signal)
--
2.17.1
On Thu, Jan 30, 2020 at 11:50:28AM +0530, [email protected] wrote:
> From: Madhuparna Bhowmik <[email protected]>
>
> This patch fixes the following sparse error:
> kernel/exit.c:627:25: error: incompatible types in comparison expression
>
> And the following warning:
> kernel/exit.c:626:40: warning: incorrect type in assignment
>
> Signed-off-by: Madhuparna Bhowmik <[email protected]>
I think the previous version was already fine but hopefully
RCU_INIT_POINTER() really saves some overhead. In any case:
Acked-by: Christian Brauner <[email protected]>
On 01/30, Christian Brauner wrote:
>
> On Thu, Jan 30, 2020 at 11:50:28AM +0530, [email protected] wrote:
> > From: Madhuparna Bhowmik <[email protected]>
> >
> > This patch fixes the following sparse error:
> > kernel/exit.c:627:25: error: incompatible types in comparison expression
> >
> > And the following warning:
> > kernel/exit.c:626:40: warning: incorrect type in assignment
> >
> > Signed-off-by: Madhuparna Bhowmik <[email protected]>
>
> I think the previous version was already fine but hopefully
> RCU_INIT_POINTER() really saves some overhead. In any case:
It is not about overhead, RCU_INIT_POINTER() documents the fact that we
didn't make any changes to the new parent, we only need to change the
pointer.
And btw, I don't really understand the __rcu annotations. Say, according
to sparse this code is wrong:
int __rcu *P;
void func(int *p)
{
P = p;
}
OK, although quite possibly it is fine.
However, this code
int __rcu *P;
void func(int __rcu *p)
{
*p = 10;
P = p;
}
is almost certainly wrong but sparse is happy, asn is the same.
> Acked-by: Christian Brauner <[email protected]>
Acked-by: Oleg Nesterov <[email protected]>
On January 30, 2020 12:33:41 PM GMT+01:00, Oleg Nesterov <[email protected]> wrote:
>On 01/30, Christian Brauner wrote:
>>
>> On Thu, Jan 30, 2020 at 11:50:28AM +0530,
>[email protected] wrote:
>> > From: Madhuparna Bhowmik <[email protected]>
>> >
>> > This patch fixes the following sparse error:
>> > kernel/exit.c:627:25: error: incompatible types in comparison
>expression
>> >
>> > And the following warning:
>> > kernel/exit.c:626:40: warning: incorrect type in assignment
>> >
>> > Signed-off-by: Madhuparna Bhowmik <[email protected]>
>>
>> I think the previous version was already fine but hopefully
>> RCU_INIT_POINTER() really saves some overhead. In any case:
>
>It is not about overhead, RCU_INIT_POINTER() documents the fact that we
>didn't make any changes to the new parent, we only need to change the
>pointer.
Right, I wasn't complaining. RCU_INIT_POINTER() claims that it has less overhead than rcu_assign_pointer().
So that is an additional argument for it.
>
>And btw, I don't really understand the __rcu annotations. Say,
>according
>to sparse this code is wrong:
>
> int __rcu *P;
>
> void func(int *p)
> {
> P = p;
> }
>
>OK, although quite possibly it is fine.
>
>However, this code
>
> int __rcu *P;
>
> void func(int __rcu *p)
> {
> *p = 10;
> P = p;
> }
>
>is almost certainly wrong but sparse is happy, asn is the same.
That's more an argument to fix sparse I guess?
The annotations themselves are rather useful I think.
They at least help me when reading the code.
It's not that rcu lifetimes are trivial and anything that helps remind me that an object wants rcu semantics I'm happy to take it. :)
>
>
>> Acked-by: Christian Brauner <[email protected]>
>
>Acked-by: Oleg Nesterov <[email protected]>
On Thu, Jan 30, 2020 at 12:45:26PM +0100, Christian Brauner wrote:
> On January 30, 2020 12:33:41 PM GMT+01:00, Oleg Nesterov <[email protected]> wrote:
> >On 01/30, Christian Brauner wrote:
> >>
> >> On Thu, Jan 30, 2020 at 11:50:28AM +0530,
> >[email protected] wrote:
> >> > From: Madhuparna Bhowmik <[email protected]>
> >> >
> >> > This patch fixes the following sparse error:
> >> > kernel/exit.c:627:25: error: incompatible types in comparison
> >expression
> >> >
> >> > And the following warning:
> >> > kernel/exit.c:626:40: warning: incorrect type in assignment
> >> >
> >> > Signed-off-by: Madhuparna Bhowmik <[email protected]>
> >>
> >> I think the previous version was already fine but hopefully
> >> RCU_INIT_POINTER() really saves some overhead. In any case:
> >
> >It is not about overhead, RCU_INIT_POINTER() documents the fact that we
> >didn't make any changes to the new parent, we only need to change the
> >pointer.
>
> Right, I wasn't complaining. RCU_INIT_POINTER() claims that it has less overhead than rcu_assign_pointer().
> So that is an additional argument for it.
>
> >
> >And btw, I don't really understand the __rcu annotations. Say,
> >according
> >to sparse this code is wrong:
> >
> > int __rcu *P;
> >
> > void func(int *p)
> > {
> > P = p;
> > }
> >
> >OK, although quite possibly it is fine.
> >
> >However, this code
> >
> > int __rcu *P;
> >
> > void func(int __rcu *p)
> > {
> > *p = 10;
> > P = p;
> > }
> >
> >is almost certainly wrong but sparse is happy, asn is the same.
>
> That's more an argument to fix sparse I guess?
> The annotations themselves are rather useful I think.
> They at least help me when reading the code.
> It's not that rcu lifetimes are trivial and anything that helps remind me that an object wants rcu semantics I'm happy to take it. :)
>
> >
> >
> >> Acked-by: Christian Brauner <[email protected]>
> >
> >Acked-by: Oleg Nesterov <[email protected]>
Thanks, applied for post -rc1.
Christian