lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 26 Jun 2017 17:57:38 +0800 From: jeffy <jeffy.chen@...k-chips.com> To: Thomas Gleixner <tglx@...utronix.de> CC: linux-kernel@...r.kernel.org, briannorris@...omium.org, dianders@...omium.org, tfiga@...omium.org Subject: Re: [RFC PATCH] genirq: Avoid unnecessary low level irq function calls Hi Thomas, thanx for your comments. On 06/26/2017 04:20 PM, Thomas Gleixner wrote: > Jeffy, > > On Mon, 26 Jun 2017, Jeffy Chen wrote: >> void irq_enable(struct irq_desc *desc) >> { >> - irq_state_clr_disabled(desc); >> - if (desc->irq_data.chip->irq_enable) >> - desc->irq_data.chip->irq_enable(&desc->irq_data); >> - else >> - desc->irq_data.chip->irq_unmask(&desc->irq_data); >> - irq_state_clr_masked(desc); >> + if (irqd_is_started(&desc->irq_data) && >> + !irqd_irq_disabled(&desc->irq_data)) { >> + unmask_irq(desc); > > I'm having a hard time to understand the logic here. > > if (started && !disabled) > unmask() > > That does not make any sense. If you need that to work around a state > inconsistency then it needs to be addressed there and not worked around > here. right, we already set disabled state in desc_set_defaults, so the disabled state should be consistency here. so just if(!disabled) unmask() else enable() ? and i saw we didn't set masked state in desc_set_defaults, i'll send a patch for that, and remove unmask_irq's started check too. > >> + } else { >> + irq_state_clr_disabled(desc); >> + if (desc->irq_data.chip->irq_enable) { >> + desc->irq_data.chip->irq_enable(&desc->irq_data); >> + irq_state_clr_masked(desc); >> + } else { >> + unmask_irq(desc); >> + } >> + } >> } >> >> static void __irq_disable(struct irq_desc *desc, bool mask) >> { >> - irq_state_set_disabled(desc); >> - if (desc->irq_data.chip->irq_disable) { >> - desc->irq_data.chip->irq_disable(&desc->irq_data); >> - irq_state_set_masked(desc); >> - } else if (mask) { >> - mask_irq(desc); >> + if (irqd_irq_disabled(&desc->irq_data)) { >> + if (mask) >> + mask_irq(desc); >> + } else { >> + irq_state_set_disabled(desc); >> + if (desc->irq_data.chip->irq_disable) { >> + desc->irq_data.chip->irq_disable(&desc->irq_data); >> + irq_state_set_masked(desc); >> + } else if (mask) { >> + mask_irq(desc); >> + } >> } >> } >> >> @@ -311,18 +322,21 @@ void irq_percpu_disable(struct irq_desc *desc, unsigned int cpu) >> >> static inline void mask_ack_irq(struct irq_desc *desc) >> { >> - if (desc->irq_data.chip->irq_mask_ack) >> + if (desc->irq_data.chip->irq_mask_ack) { >> desc->irq_data.chip->irq_mask_ack(&desc->irq_data); >> - else { >> - desc->irq_data.chip->irq_mask(&desc->irq_data); >> + irq_state_set_masked(desc); >> + } else { >> + mask_irq(desc); >> if (desc->irq_data.chip->irq_ack) >> desc->irq_data.chip->irq_ack(&desc->irq_data); >> } >> - irq_state_set_masked(desc); >> } >> >> void mask_irq(struct irq_desc *desc) >> { >> + if (irqd_irq_masked(&desc->irq_data)) >> + return; >> + >> if (desc->irq_data.chip->irq_mask) { >> desc->irq_data.chip->irq_mask(&desc->irq_data); >> irq_state_set_masked(desc); >> @@ -331,6 +345,10 @@ void mask_irq(struct irq_desc *desc) >> >> void unmask_irq(struct irq_desc *desc) >> { >> + if (irqd_is_started(&desc->irq_data) && >> + !irqd_irq_masked(&desc->irq_data)) >> + return; > > Ditto. > > Thanks, > > tglx > > >
Powered by blists - more mailing lists