linux-kernel - Re: [PATCH next] wifi: rtw89: unlock on error path in rtw89_ops_unassign_vif

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: <9729c2f5-2df5-4468-a4da-cd31f4332b4c@stanley.mountain>
Date: Wed, 23 Oct 2024 13:06:18 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Kalle Valo <kvalo@...nel.org>
Cc: Ping-Ke Shih <pkshih@...ltek.com>,
	Zong-Zhe Yang <kevin_yang@...ltek.com>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH next] wifi: rtw89: unlock on error path in
 rtw89_ops_unassign_vif_chanctx()

On Wed, Oct 23, 2024 at 12:38:38PM +0300, Kalle Valo wrote:
> Dan Carpenter <dan.carpenter@...aro.org> writes:
> 
> > On Tue, Oct 22, 2024 at 03:32:23AM +0000, Ping-Ke Shih wrote:
> >
> >> Zong-Zhe Yang <kevin_yang@...ltek.com> wrote:
> >> > Dan Carpenter <dan.carpenter@...aro.org> wrote:
> >> > >
> >> > > [...]
> >> > >
> >> > > @@ -1373,6 +1373,7 @@ static void rtw89_ops_unassign_vif_chanctx(struct ieee80211_hw
> >> > > *hw,
> >> > >
> >> > >         rtwvif_link = rtwvif->links[link_conf->link_id];
> >> > >         if (unlikely(!rtwvif_link)) {
> >> > > +               mutex_unlock(&rtwdev->mutex);
> >> > >                 rtw89_err(rtwdev,
> >> > >                           "%s: rtwvif link (link_id %u) is not active\n",
> >> > >                           __func__, link_conf->link_id);
> >> > >
> >> > 
> >> > Acked-by: Zong-Zhe Yang <kevin_yang@...ltek.com>
> >> > 
> >> 
> >> Thanks for the ack. 
> >> 
> >> Acked-by is often used by the maintainer, so I will change it to Reviewed-by
> >> during committing. 
> >
> > To me Acked by just means you're okay with the patch.  When I use it, it means I
> > don't feel qualified or interested enough to do a full review.  For example, if
> > I complain about a v1 patch and they fix my issue in v2 then I like to say that
> > I'm okay with it.  In that case I'll use Reviewed-by for a full review or Acked
> > by if the bits that I care about are okay.  I don't like to complain and then
> > just go silent.
> >
> > In the end, it doesn't make any difference.  You'll get CC'd on bug reports to
> > do with the patch and you'll potentially feel bad for not spotting the bug, I
> > guess.
> 
> I have understood that Acked-by should be only used by the corresponding
> maintainers and the documentation seems to say the same:
> 
> https://docs.kernel.org/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

"If a person was not directly involved in the preparation or handling of a patch
but wishes to signify and record their approval of it then they can ask to have
an Acked-by: line added to the patch’s changelog."

The documentation does say that it's also often used by maintainers for
approving part of a patchset.  But to me, it's the "partial" which is the more
important word in that sentence.  I haven't reviewed the whole patch.

> 
> The reason I ask non-maintainers avoid using Acked-by is that it messes
> our patchwork listings (it counts both Acked-by and Reviewed-by tags).
> 
> -- 
> https://patchwork.kernel.org/project/linux-wireless/list/

Huh.  I wasn't aware that anything really differentiated between Acks and
Reviews.  That does make things more complicated.

I rarely do Acks, but when I do it's because I'm outside of a subsystem I'm
familiar with.  I would say LGTM and leave it at that, except other people want
proper tags.  Probably they won't insist on proper tags from me though so it's
fine.

regards,
dan carpenter