Re: [RFC PATCH v3] genirq/affinity: Create and transfer more irq desc info by a new structure

[Dou Liyang]

Hi Thomas,

On 2018/11/29 6:03, Thomas Gleixner wrote:

> > +		affi_desc = kcalloc(nvec, sizeof(*affi_desc), GFP_KERNEL);
>
> Why do you want to do that separate allocation here? Just let

I thought the irq_create_affinity_desc() also can be called by other 
functions which may convert cpumasks to irq_affinity_desc, such as
__devm_irq_alloc_descs().

Now, I know I was wrong, will modify it.

> irq_create_affinity_masks() allocate an array of affinity descriptors and
> use that. There is no point in copying that stuff over and over. Setting
> the flag field can be done in the existing function as well.

> Can you please change the function signature and fixup the callers, if
> there are any of them? Copying this over and over is horrible.

I have searched, no one calls __devm_irq_alloc_descs, it may be called
by some users' own modules or drives.

yes, I will change it.

> struct irq_affinity_desc {
> 	struct cpumask	masks;
> 	unsigned int	managed : 1; > };

yes, BTW, If the following is more fit for irq_affinity_desc:

s/masks/mask/
s/managed/is_managed/

>

> You can spare a lot of pointless churn by just keeping the 'affinity' name
> and only changing the struct type. The compiler will catch all places which
> need to be fixed and 'affinity' is generic enough to be used with the new
> struct type as well. As Bjorn said, even 'masks' is fine.

Yes, I see

Thanks,
	dou