Re: [PATCH 3/4] net: korina: staging: octeon: Use DEV_STAT_INC()

[Simon Horman]

On Sat, Nov 09, 2024 at 05:20:24PM +0800, zhangheng wrote:
> syzbot/KCSAN reported that races happen when multiple CPUs updating
> dev->stats.tx_error concurrently. Adopt SMP safe DEV_STATS_INC() to
> update the dev->stats fields.
> 
> Signed-off-by: zhangheng <zhangheng@xxxxxxxxxx>

Hi,

I'm unsure what happened to the other 3 patches of the series
(and the cover letter?), but only this patch seems to be known
to lore.

This path looks like it is a fix for Networking, and thus should
be targeted at the net tree.

	Subject: [PATCH net] ...

And, looking at git history, I think just "net: korina: " would be an
appropriate prefix for this patch. Along with a slightly more descriptive
subject. Maybe:

	Subject: [PATCH net] net: korina: Use DEV_STAT_INC() to avoid race

Next, is there a stack trace available? If so, it would be nice
to include it, succinctly, in the patch description.

And if there there is a public syzbot/KCSAN report it would be nice
to include a link to it.

As for the changes, you mention that the patch fixes a race wrt to
tx_error. But the patch does more than that. Are the other changes also
strictly necessary? If so, I think that should be mentioned in the patch
description. And I'd suggest that any changes that are not strictly
necessary as a bug-fix should be handled via a follow-up patch targeted
at net-next.

As a fix there should be a Fixes tag immediately above your Signed-off-by
line (no blank line in between). It should cite the commit where the bug
was introduced, typically the first commit where it manifests for users.

Lastly, information on the process for Networking patches can be
found here: https://docs.kernel.org/process/maintainer-netdev.html

...

-- 
pw-bot: changes-requested