Re: [PATCH 0/3] task: Making tasks on the runqueue rcu protected
On Tue, Sep 03, 2019 at 08:44:40AM -0700, Linus Torvalds wrote: > That said, it won't affect any of the core architectures much, because > smp_store_release() isn't that expensive (it's just a compiler barrier > on x86, it's a cheap instruction on arm64, and it should be very cheap > on any other architecture too unless they do insane things - even on > powerpc, which is about the worst case for any barriers, it's just an > lwsync). Right, x86/s390/Sparc it's a compiler barrier, ARM64 has a store-release op which is relatively cheap, Power has an LWSYNC, but the rest does store-release with smp_mb() -- and this includes ARM.
Re: [PATCH 0/3] task: Making tasks on the runqueue rcu protected
On Mon, Sep 2, 2019 at 9:50 PM Eric W. Biederman wrote: > > I have split this work into 3 simple patches, so the code is straight > forward to review and so that if any mistakes slip in it is easy to > bisect them. In the process of review what it takes to remove > task_rcu_dereference I found yet another user of tasks on the > runqueue in rcu context; the rcuwait_event code. That code only needs > it now unnecessary limits removed. Looks very good to me. I think PeterZ is right that the rcu_assign_pointer() in [PATCH 2/3] could be a RCU_INIT_POINTER() due to condition #3 in the RCU_INIT_POINTER rules. The initialization of the pointer value simply has nothing to do with what the pointer points to - we're just switching it to another case. That said, it won't affect any of the core architectures much, because smp_store_release() isn't that expensive (it's just a compiler barrier on x86, it's a cheap instruction on arm64, and it should be very cheap on any other architecture too unless they do insane things - even on powerpc, which is about the worst case for any barriers, it's just an lwsync). It might be good to have Paul _look_ at it, and because of the minimal performance impact I don't worry about it too much if it happens later, but it should be something we keep in mind. Linus
Re: [PATCH 0/3] task: Making tasks on the runqueue rcu protected
On 09/02, Eric W. Biederman wrote: > > Oleg do you have any issues with this code? OK, let it be refcount_t, I agree it looks more readable. > Eric W. Biederman (3): > task: Add a count of task rcu users > task: RCU protect tasks on the runqueue > task: Clean house now that tasks on the runqueue are rcu protected > > include/linux/rcuwait.h| 20 +++-- > include/linux/sched.h | 5 +++- > include/linux/sched/task.h | 2 +- > kernel/exit.c | 74 > -- > kernel/fork.c | 8 +++-- > kernel/sched/core.c| 7 +++-- > kernel/sched/fair.c| 2 +- > kernel/sched/membarrier.c | 4 +-- > 8 files changed, 27 insertions(+), 95 deletions(-) Reviewed-by: Oleg Nesterov
[PATCH 0/3] task: Making tasks on the runqueue rcu protected
I have split this work into 3 simple patches, so the code is straight forward to review and so that if any mistakes slip in it is easy to bisect them. In the process of review what it takes to remove task_rcu_dereference I found yet another user of tasks on the runqueue in rcu context; the rcuwait_event code. That code only needs it now unnecessary limits removed. I have lightly tested it, and read through everything I can think of that might be an issue. Peter would this be a good fit for your scheduler tree? If not I will toss it onto a branch someplace and send it to Linus when the merge window opens. Oleg do you have any issues with this code? Eric W. Biederman (3): task: Add a count of task rcu users task: RCU protect tasks on the runqueue task: Clean house now that tasks on the runqueue are rcu protected include/linux/rcuwait.h| 20 +++-- include/linux/sched.h | 5 +++- include/linux/sched/task.h | 2 +- kernel/exit.c | 74 -- kernel/fork.c | 8 +++-- kernel/sched/core.c| 7 +++-- kernel/sched/fair.c| 2 +- kernel/sched/membarrier.c | 4 +-- 8 files changed, 27 insertions(+), 95 deletions(-) Eric
