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: <CADUfDZr_ecu-Vap_oPLPUJTiCaeUftErazDj702Ld2KDwvGUbQ@mail.gmail.com>
Date:   Wed, 21 Dec 2022 08:48:17 -0800
From:   Caleb Sander <csander@...estorage.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     Ariel Elior <aelior@...vell.com>,
        Manish Chopra <manishc@...vell.com>, netdev@...r.kernel.org,
        Joern Engel <joern@...estorage.com>
Subject: Re: [PATCH] qed: allow sleep in qed_mcp_trace_dump()

On Tue, Dec 20, 2022 at 1:55 AM Paolo Abeni <pabeni@...hat.com> wrote:
>
> On Sat, 2022-12-17 at 10:56 -0700, Caleb Sander wrote:
> > By default, qed_mcp_cmd_and_union() waits for 10us at a time
> > in a loop that can run 500K times, so calls to qed_mcp_nvm_rd_cmd()
> > may block the current thread for over 5s.
> > We observed thread scheduling delays of over 700ms in production,
> > with stacktraces pointing to this code as the culprit.
>
> IMHO this is material eligible for the net tree...
>
> >
> > qed_mcp_trace_dump() is called from ethtool, so sleeping is permitted.
> > It already can sleep in qed_mcp_halt(), which calls qed_mcp_cmd().
> > Add a "can sleep" parameter to qed_find_nvram_image() and
> > qed_nvram_read() so they can sleep during qed_mcp_trace_dump().
> > qed_mcp_trace_get_meta_info() and qed_mcp_trace_read_meta(),
> > called only by qed_mcp_trace_dump(), allow these functions to sleep.
> > It's not clear to me that the other caller (qed_grc_dump_mcp_hw_dump())
> > can sleep, so it keeps b_can_sleep set to false.
>
> ...but we need a suitable Fixes tag here. Please repost specifying the
> target tree into the subject and adding the relevant tag, thanks!

Sure, I can do that, but I would like to get some sign-off from the
driver authors.
The last time we attempted to fix this bug, we were told our change
could cause the driver to sleep in atomic contexts. So it would be great to hear
from QLogic (now Marvell) whether this fix is acceptable.

Thanks,
Caleb

Powered by blists - more mailing lists