On 04/24/2014 09:07 AM, Peter Zijlstra wrote:
> On Wed, Apr 23, 2014 at 09:00:36PM +0200, Denys Vlasenko wrote:
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 9cae286..08dd220 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4255,6 +4255,9 @@ EXPORT_SYMBOL_GPL(yield_to);
>> */
>> void __sched io_schedule(void)
>> {
>> +#ifdef CONFIG_NO_HZ_COMMON
>> + int cpu_on_entry = smp_processor_id();
>> +#endif
>> struct rq *rq = raw_rq();
>>
>> delayacct_blkio_start();
>> @@ -4263,13 +4266,23 @@ void __sched io_schedule(void)
>> current->in_iowait = 1;
>> schedule();
>> current->in_iowait = 0;
>> +#ifdef CONFIG_NO_HZ_COMMON
>> + if (atomic_dec_and_test(&rq->nr_iowait)) {
>> + if (smp_processor_id() != cpu_on_entry)
>> + tick_nohz_iowait_to_idle(cpu_on_entry);
>> + }
>> +#else
>> atomic_dec(&rq->nr_iowait);
>> +#endif
>> delayacct_blkio_end();
>> }
>> EXPORT_SYMBOL(io_schedule);
>>
>> long __sched io_schedule_timeout(long timeout)
>> {
>> +#ifdef CONFIG_NO_HZ_COMMON
>> + int cpu_on_entry = smp_processor_id();
>> +#endif
>> struct rq *rq = raw_rq();
>> long ret;
>>
>> @@ -4279,7 +4292,14 @@ long __sched io_schedule_timeout(long timeout)
>> current->in_iowait = 1;
>> ret = schedule_timeout(timeout);
>> current->in_iowait = 0;
>> +#ifdef CONFIG_NO_HZ_COMMON
>> + if (atomic_dec_and_test(&rq->nr_iowait)) {
>> + if (smp_processor_id() != cpu_on_entry)
>> + tick_nohz_iowait_to_idle(cpu_on_entry);
>> + }
>> +#else
>> atomic_dec(&rq->nr_iowait);
>> +#endif
>> delayacct_blkio_end();
>> return ret;
>> }
>
> Why do you insist on writing the same (buggy, see later) code twice?
I don't insist on such a thing. I am modifying existing code
which already has these nearly-identical functions.
I agree with you that they beg for common code being moved into
a shared helper function.
> Its buggy because the smp_processor_id() is used in preemptible context,
> its further buggy because the raw_rq() does it again and could get a rq
> on a different cpu.
>
> What you want is something like:
>
> static inline void io_wait_start(struct rq *rq)
> {
> atomic_inc(&rq->nr_iowait);
> current->in_iowait = 1;
> }
>
> static inline void io_wait_end(struct rq *rq)
> {
> current->in_iowait = 0;
> #ifdef CONFIG_NO_HZ_COMMON
> if (atomic_dec_and_test(&rq->nr_iowait) &&
> cpu_of(rq) != raw_smp_processor_id()) {
> tick_nohz_iowait_end(cpu_of(rq));
> }
> #else
> atomic_dec(&rq->nr_iowait);
> #endif
> }
Thanks, will use this in the next patchset.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
