From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755450AbbCCBQe (ORCPT ); Mon, 2 Mar 2015 20:16:34 -0500 Received: from linuxhacker.ru ([217.76.32.60]:40421 "EHLO fiona.linuxhacker.ru" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1752327AbbCCBQd convert rfc822-to-8bit (ORCPT ); Mon, 2 Mar 2015 20:16:33 -0500 Subject: Re: [PATCH 05/16] staging/lustre: fix up obsolete cpu function usage. Mime-Version: 1.0 (Apple Message framework v1283) Content-Type: text/plain; charset=us-ascii From: Oleg Drokin In-Reply-To: <87twy3j7fo.fsf@rustcorp.com.au> Date: Mon, 2 Mar 2015 20:16:23 -0500 Cc: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8BIT Message-Id: <2CED6DE1-CF72-42D7-AEB6-247E4FB20539@linuxhacker.ru> References: <1425296150-4722-1-git-send-email-rusty@rustcorp.com.au> <1425296150-4722-5-git-send-email-rusty@rustcorp.com.au> <74B1B5A4-D3CE-4A7A-A09B-6B755623ADA4@linuxhacker.ru> <87twy3j7fo.fsf@rustcorp.com.au> To: Rusty Russell X-Mailer: Apple Mail (2.1283) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mar 2, 2015, at 6:39 PM, Rusty Russell wrote: >> if (i == NR_CPUS) >> blah; >> >> when we are replacing for_each_cpu_mask with for_each_cpu, >> what do we check the counter against now to see that the entire loop was executed >> and we did not exit prematurely? nr_cpu_ids? > You want >= nr_cpu_ids here. Aha, Thanks! >> Also I assume we still want to get rid of direct cpumask assignments like >>> mask = *cpumask_of_node(cpu_to_node(index)); > > Yes, but this code is wrong anyway: > > mask = *cpumask_of_node(cpu_to_node(index)); > for (i = max; i < num_online_cpus(); i++) > cpumask_clear_cpu(i, &mask); > > *Never* iterate to num_online_cpus(). eg. if cpus 0 and 3 are online, > num_online_cpus() == 2. I'm not sure what this code is doing, but it's > not doing it well :) Oh my, I don't know how I have not seen it sooner. I think I developed an idea of what it is trying to do. Thanks for highlighting it. So there are 7 more users like this outside of Lustre in the kernel then, I'll try for a patch: ./drivers/scsi/hpsa.c: for (i = 0; i < num_online_cpus(); i++) { -- this one seems to be just opencoding for_each_cpu, though ./arch/um/kernel/smp.c: for (i = 0; i < num_online_cpus(); i++) { -- I wonder if UML is able to have discontiguous cpus up? ./arch/sh/include/asm/mmu_context.h: for (i = 0; i < num_online_cpus(); i++) -- this seems to be the same bug we have. ./arch/sh/kernel/smp.c: for (i = 0; i < num_online_cpus(); i++) -- this and the two below it too ./arch/sh/kernel/smp.c: for (i = 0; i < num_online_cpus(); i++) ./arch/sh/kernel/smp.c: for (i = 0; i < num_online_cpus(); i++) ./arch/m32r/kernel/smpboot.c: for (cpu_id = 0 ; cpu_id < num_online_cpus() ; cpu_id++) -- this also is buggy, though it's just an info print. Bye, Oleg