linux-kernel - Re: [PATCH 1/2] libsas: Don't process sas events in static works

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPcyv4idOqGpdFt=FKRPs0gAqYc8pOv+=gL3PGN0Wgp6maUcMw@mail.gmail.com>
Date:   Sat, 20 May 2017 20:44:16 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Yijing Wang <wangyijing@...wei.com>
Cc:     "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        chenqilin2@...wei.com, hare@...e.com,
        linux-scsi <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        chenxiang66@...ilicon.com, huangdaode@...ilicon.com,
        wangkefeng.wang@...wei.com, zhaohongjiang@...wei.com,
        dingtianhong@...wei.com, guohanjun@...wei.com,
        John Garry <john.garry@...wei.com>,
        Wei Fang <fangwei1@...wei.com>, yanaijie@...wei.com,
        Christoph Hellwig <hch@....de>,
        Yousong He <heyousong@...wei.com>
Subject: Re: [PATCH 1/2] libsas: Don't process sas events in static works

On Fri, May 19, 2017 at 11:39 PM, Yijing Wang <wangyijing@...wei.com> wrote:
> Now libsas hotplug work is static, LLDD driver queue
> the hotplug work into shost->work_q. If LLDD driver
> burst post lots hotplug events to libsas, the hotplug
> events may pending in the workqueue like
>
> shost->work_q
> new work[PORTE_BYTES_DMAED] --> |[PHYE_LOSS_OF_SIGNAL][PORTE_BYTES_DMAED] -> processing
>                                 |<-------wait worker to process-------->|
> In this case, a new PORTE_BYTES_DMAED event coming, libsas try to queue it
> to shost->work_q, but this work is already pending, so it would be lost.
> Finally, libsas delete the related sas port and sas devices, but LLDD driver
> expect libsas add the sas port and devices(last sas event).
>
> This patch remove the static defined hotplug work, and use dynamic work to
> avoid missing hotplug events.

If we go this route we don't even need:

sas_port_event_fns
sas_phy_event_fns
sas_ha_event_fns

...just specify the target routine directly to INIT_WORK() and remove
the indirection.

I also think for safety this should use a mempool that guarantees that
events can continue to be processed under system memory pressure.
Also, have you considered the case when a broken phy starts throwing a
constant stream of events? Is there a point at which libsas should
stop queuing events and disable the phy?