Re: [PATCH 6/8] x86/apic: Avoid useless scanning thru a cpumask inassign_irq_vector()

[Suresh Siddha]

On Thu, 2012-06-07 at 15:15 +0200, Alexander Gordeev wrote:
> In case of static vector allocation domains (i.e. flat) if all vector
> numbers are exhausted, an attempt to assign a new vector will lead to
> useless scans through all CPUs in the cpumask, even though it is known
> that each new pass would fail. Make this corner case less painful by
> letting report whether the vector allocation domain depends on passed
> arguments or not and stop scanning early.
> 
> The same could have been achived by introducing a static flag to the
> apic operations. But let's allow vector_allocation_domain() have more
> intelligence here and decide dynamically, in case we would need it in
> the future.

Acked-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>

> 
> Signed-off-by: Alexander Gordeev <agordeev@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/apic.h      |    8 +++++---
>  arch/x86/kernel/apic/apic_noop.c |    3 ++-
>  arch/x86/kernel/apic/io_apic.c   |   12 +++++++++---
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index feb2dbd..e3fecd5 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -306,7 +306,7 @@ struct apic {
>  	unsigned long (*check_apicid_used)(physid_mask_t *map, int apicid);
>  	unsigned long (*check_apicid_present)(int apicid);
>  
> -	void (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
> +	bool (*vector_allocation_domain)(int cpu, struct cpumask *retmask);
>  	void (*init_apic_ldr)(void);
>  
>  	void (*ioapic_phys_id_map)(physid_mask_t *phys_map, physid_mask_t *retmap);
> @@ -615,7 +615,7 @@ extern unsigned int
>  default_cpu_mask_to_apicid_and(const struct cpumask *cpumask,
>  			       const struct cpumask *andmask);
>  
> -static inline void
> +static inline bool
>  flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
>  {
>  	/* Careful. Some cpus do not strictly honor the set of cpus
> @@ -628,12 +628,14 @@ flat_vector_allocation_domain(int cpu, struct cpumask *retmask)
>  	 */
>  	cpumask_clear(retmask);
>  	cpumask_bits(retmask)[0] = APIC_ALL_CPUS;
> +	return false;
>  }
>  
> -static inline void
> +static inline bool
>  default_vector_allocation_domain(int cpu, struct cpumask *retmask)
>  {
>  	cpumask_copy(retmask, cpumask_of(cpu));
> +	return true;
>  }
>  
>  static inline unsigned long default_check_apicid_used(physid_mask_t *map, int apicid)
> diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
> index 3e43cf5..ac9edf2 100644
> --- a/arch/x86/kernel/apic/apic_noop.c
> +++ b/arch/x86/kernel/apic/apic_noop.c
> @@ -100,11 +100,12 @@ static unsigned long noop_check_apicid_present(int bit)
>  	return physid_isset(bit, phys_cpu_present_map);
>  }
>  
> -static void noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
> +static bool noop_vector_allocation_domain(int cpu, struct cpumask *retmask)
>  {
>  	if (cpu != 0)
>  		pr_warning("APIC: Vector allocated for non-BSP cpu\n");
>  	cpumask_copy(retmask, cpumask_of(cpu));
> +	return true;
>  }
>  
>  static u32 noop_apic_read(u32 reg)
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index 05af3d3..4061a7d 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1137,8 +1137,9 @@ __assign_irq_vector(int irq, struct irq_cfg *cfg, const struct cpumask *mask)
>  	for_each_cpu_and(cpu, mask, cpu_online_mask) {
>  		int new_cpu;
>  		int vector, offset;
> +		bool more_domains;
>  
> -		apic->vector_allocation_domain(cpu, tmp_mask);
> +		more_domains = apic->vector_allocation_domain(cpu, tmp_mask);
>  
>  		if (cpumask_subset(tmp_mask, cfg->domain)) {
>  			free_cpumask_var(tmp_mask);
> @@ -1153,8 +1154,13 @@ next:
>  			offset = (offset + 1) % 16;
>  			vector = FIRST_EXTERNAL_VECTOR + offset;
>  		}
> -		if (unlikely(current_vector == vector))
> -			continue;
> +
> +		if (unlikely(current_vector == vector)) {
> +			if (more_domains)
> +				continue;
> +			else
> +				break;
> +		}
>  
>  		if (test_bit(vector, used_vectors))
>  			goto next;
> -- 
> 1.7.7.6
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/