All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fabio Aiuto <fabioaiuto83@gmail.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	Martin Kaiser <martin@kaiser.cx>,
	Phillip Potter <phil@philpotter.co.uk>,
	Michael Straube <straube.linux@gmail.com>,
	"Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
	Pavel Skripkin <paskripkin@gmail.com>
Subject: Re: [PATCH v2] staging: rtl8723bs: remove possible deadlock when disconnect
Date: Sat, 11 Sep 2021 12:53:08 +0200	[thread overview]
Message-ID: <20210911105307.GB1407@agape.jhs> (raw)
In-Reply-To: <c730848c-3c8d-1e49-fa74-b956400a5d3d@redhat.com>

Hello Hans,

On Fri, Sep 10, 2021 at 11:06:58PM +0200, Hans de Goede wrote:
> Hi,

<snip>

> 
> Thank you for your work on this. Overall this looks good.
> 
> I have one remark, since now you are relying on the
> sta->sleep_q.lock to protect the sleep_q data, you also
> need to update the sleep_q accesses in rtw_free_stainfo()
> specifically you need to add a spin_{lock,unlock}_bh(psta->sleep_q.lock)
> around these lines:
> 
> 
>         rtw_free_xmitframe_queue(pxmitpriv, &psta->sleep_q);
>         psta->sleepq_len = 0;
> 
> 
> Note there also is a:
> 
>         spin_lock_bh(&pxmitpriv->lock);
> 
> Just above this which needs to be pushed down to below the
> block which takes the psta->sleep_q.lock, so that the entire
> thing ends up looking like this:
> 
> 	spin_lock_bh(&psta->sleep_q.lock);
>         rtw_free_xmitframe_queue(pxmitpriv, &psta->sleep_q);
>         psta->sleepq_len = 0;
> 	spin_unlock_bh(&psta->sleep_q.lock);
> 
>         spin_lock_bh(&pxmitpriv->lock);
> 
> Other then that this patch looks good, thanks.
> 
> Regards,
> 
> Hans
> 

good catch, thanks a lot for review.

Greg, I see that this one is not in staging-next,
would you drop it so I can send a new one?
Or shall I send a Fixes: patch?

thank you all,

fabio

  reply	other threads:[~2021-09-11 10:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02  9:35 [PATCH v2] staging: rtl8723bs: remove possible deadlock when disconnect Fabio Aiuto
2021-09-10 21:06 ` Hans de Goede
2021-09-11 10:53   ` Fabio Aiuto [this message]
2021-09-11 11:33     ` Hans de Goede
2021-09-11 15:08       ` Fabio Aiuto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210911105307.GB1407@agape.jhs \
    --to=fabioaiuto83@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=fmdefrancesco@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    --cc=straube.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.