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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190807091513.m2aqfbk32y6qb343@wunner.de>
Date:   Wed, 7 Aug 2019 11:15:13 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Xiongfeng Wang <wangxiongfeng2@...wei.com>
Cc:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org, yaohongbo@...wei.com,
        guohanjun@...wei.com, huawei.libin@...wei.com
Subject: Re: [RFC PATCH] pciehp: use completion to wait irq_thread
 'pciehp_ist'

On Wed, Aug 07, 2019 at 04:28:32PM +0800, Xiongfeng Wang wrote:
> On 2019/8/6 15:24, Lukas Wunner wrote:
> > I'd suggest something like the below instead, could you give it a whirl
> > and see if it reliably fixes the issue for you?
> 
> I tested the below patch. It can fix the issue.

Thank you!  I'll submit it as a proper patch then.


> I am not sure whether the following sequence will be a problem.
> * pciehp_ist() is running, and 'ctrl->pending_events' is cleared
> * a request to disable the slot is submitted via sysfs.
>   'ctrl->pending_events' is set and the irq_thread 'pciehp_ist' is waken up.
>   But pciehp_ist() is running. So it doesn't take effect.
>   'ctrl->pending_events' is not cleared until next time pciehp_ist() is
>   waken up. So pciehp_sysfs_enable_slot() will wait until next
>   pciehp_ist() is waken up. I am not sure how 'irq_wake_thread()' will
>   effect the running irq_thread.

That's not a problem.  If irq_wake_thread() is called while pciehp_ist()
is running, the latter will automatically perform another iteration.
It's the same situation if an interrupt is received while pciehp_ist()
is running.

irq_wake_thread() sets the IRQTF_RUNTHREAD flag and irq_wait_for_interrupt()
checks that flag and causes irq_thread() to perform another invocation
of handler_fn(), which is pciehp_ist() in this case.

So pciehp basically treats a user request like an interrupt received from
the hardware.  It's meant to simplify the pciehp code.  But it's non-trivial
to understand because one needs to have an understanding of the genirq
code to appreciate the simplicity.

Let me know if this explanation wasn't clear enough and you have further
questions.


> How about making the process synchronous instead of waking up the
> irq_thread?

That's what we had before, but it has its own problems since it requires
locking and interaction between the IRQ thread and the sysfs entry points.

Thanks,

Lukas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ