netdev - Re: [PATCH v6 net-next 07/19] ionic: Add basic adminq support

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9a0d505b-dc43-ea2e-ae9f-5a37056651fa@pensando.io>
Date:   Fri, 30 Aug 2019 16:18:46 -0700
From:   Shannon Nelson <snelson@...sando.io>
To:     David Miller <davem@...emloft.net>, jakub.kicinski@...ronome.com
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH v6 net-next 07/19] ionic: Add basic adminq support

On 8/30/19 3:17 PM, David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@...ronome.com>
> Date: Fri, 30 Aug 2019 15:16:04 -0700
>
>> On Fri, 30 Aug 2019 12:31:07 -0700, Shannon Nelson wrote:
>>> On 8/29/19 3:52 PM, Jakub Kicinski wrote:
>>>> On Thu, 29 Aug 2019 11:27:08 -0700, Shannon Nelson wrote:
>>>>> +static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
>>>>> +{
>>>>> +	struct ionic_dev *idev = &lif->ionic->idev;
>>>>> +	struct device *dev = lif->ionic->dev;
>>>>> +
>>>>> +	if (!qcq)
>>>>> +		return;
>>>>> +
>>>>> +	ionic_debugfs_del_qcq(qcq);
>>>>> +
>>>>> +	if (!(qcq->flags & IONIC_QCQ_F_INITED))
>>>>> +		return;
>>>>> +
>>>>> +	if (qcq->flags & IONIC_QCQ_F_INTR) {
>>>>> +		ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
>>>>> +				IONIC_INTR_MASK_SET);
>>>>> +		synchronize_irq(qcq->intr.vector);
>>>>> +		devm_free_irq(dev, qcq->intr.vector, &qcq->napi);
>>>> Doesn't free_irq() basically imply synchronize_irq()?
>>> The synchronize_irq() waits for any threaded handlers to finish, while
>>> free_irq() only waits for HW handling.  This helps makes sure we don't
>>> have anything still running before we remove resources.
>> mm.. I'm no IRQ expert but it strikes me as surprising as that'd mean
>> every single driver would always have to run synchronize_irq() on
>> module exit, no?
>>
>> I see there is a kthread_stop() in __free_irq(), you sure it doesn't
>> wait for threaded IRQs?
> I'm pretty sure it does.

Yes, deeper in there are the kthread_stop() calls that make the 
synchronize_irq() unneccessary.  I'll pull it out.

Thanks,
sln