On 2015/1/20 17:31, Thomas Gleixner wrote:
> On Mon, 19 Jan 2015, Jiang Liu wrote:
>
>> Hi Thomas and Marc,
>> During working on the generic MSI support, I have some proposal
>> about reorganizing struct irq_data and struct irq_desc. The proposed
>> changes are:
>> 1) Add a pointer "struct irq_desc *" to struct irq_data, so we could
>> quickly get struct irq_desc from struct irq_data.
>> 2) Move "node" from struct irq_data into struct irq_desc, NUMA info
>> should be per-irq instead of per-chip.
>> 3) Move "affinity" from struct irq_data into struct irq_desc, NUMA info
>> should be per-irq instead of per-chip.
>> 4) Move "msi_desc" from struct irq_data into struct irq_desc. (Not sure
>> whether we should do this. Theoretically we should use
>> irq_data->handler_data to store msi_desc.)
>
> msi_desc belongs to the msi chip, while handler_data is common data.
>
> I had a look at the usage sites of handler_data. Most of them use it
> in combination with chained handlers. Some sites use it instead of
> chip data and only a few use it for some random other stuff, where the
> x86 sites (hpet, ht, iommu ...) will go away with the irqdomain
> conversion.
>
> So in the long run we should provide:
>
> irq_set_chained_handler_and_data(irq, handler, handler_data)
>
> convert everything over and finally remove the direct accessor to
> handler_data.
>
> msi_desc in a hierarchical implementation should actually be in
> chip_data, but we probably need to keep the msi_desc pointer for
> backward compability reasons.
>
>> With above change applied, struct irq_data only hosts per-chip data, and
>> struct irq_desc hosts per-irq data. What's your thoughts?
>
> I'm not so happy with exposing irqdesc to random code again. I went a
> great way to hide it from abuse.
>
> So I'd rather like to see something like this:
>
> struct irq_common_data {
> unsigned int state_use_accessors;
> unsigned int node;
> void *handler_data;
> cpumask_var_t affinity;
> };
>
> struct irq_data {
> u32 mask;
> unsigned int irq;
> unsigned long hwirq;
> struct irq_chip *chip;
> struct irq_domain *domain;
> #ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> struct irq_data *parent_data;
> #endif
> void *chip_data;
> struct msi_desc *msi_desc;
> struct irq_common_data *common_data;
> };
>
> struct irq_desc {
> struct irq_data irq_data;
> struct common_irq_data common_data;
> ...
> };
Great, will go this step by step:)
>
> Thanks,
>
> tglx
> --
> 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/
>
--
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/
