netdev - RE: [EXT] Re: [PATCH] qed: avoid spin loops in _qed_mcp_cmd_and

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: <PH0PR18MB465585F216AEC6E441B7E0E4C4869@PH0PR18MB4655.namprd18.prod.outlook.com>
Date:   Thu, 28 Oct 2021 05:47:10 +0000
From:   Ariel Elior <aelior@...vell.com>
To:     Caleb Sander <csander@...estorage.com>,
        Eric Dumazet <eric.dumazet@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "GR-everest-linux-l2@...vell.com" <GR-everest-linux-l2@...vell.com>,
        Joern Engel <joern@...estorage.com>
Subject: RE: [EXT] Re: [PATCH] qed: avoid spin loops in
 _qed_mcp_cmd_and_union()

> > On 10/27/21 2:45 PM, Caleb Sander wrote:
> > > By default, qed_mcp_cmd_and_union() sets max_retries to 500K and
> > > usecs to 10, so these loops can together delay up to 5s.
> > > We observed thread scheduling delays of over 700ms in production,
> > > with stacktraces pointing to this code as the culprit.
> > >
> > > Add calls to cond_resched() in both loops to yield the CPU if necessary.
> > >
> > > Signed-off-by: Caleb Sander <csander@...estorage.com>
> > > Reviewed-by: Joern Engel <joern@...estorage.com>
> > > ---
> > >  drivers/net/ethernet/qlogic/qed/qed_mcp.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> > > b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> > > index 24cd41567..d6944f020 100644
> > > --- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> > > +++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
> > > @@ -485,10 +485,12 @@ _qed_mcp_cmd_and_union(struct qed_hwfn
> > > *p_hwfn,
> > >
> > >               spin_unlock_bh(&p_hwfn->mcp_info->cmd_lock);
> > >
> > > -             if (QED_MB_FLAGS_IS_SET(p_mb_params, CAN_SLEEP))
> > > +             if (QED_MB_FLAGS_IS_SET(p_mb_params, CAN_SLEEP)) {
> >
> > I do not know this driver, but apparently, there is this CAN_SLEEP
> > test hinting about being able to sleep.
Hi,
Indeed this function sends messages to the management FW, and may
be invoked both from atomic contexts and from non atomic ones.
CAN_SLEEP indicated whether it is permissible in the context from which
it was invoked to sleep.