Re: Linking Patchwork with Sashiko?

[Roman Gushchin <roman.gushchin@linux.dev>]

Mauro Carvalho Chehab <mchehab+huawei@kernel.org> writes:

> Hi Derek/Roman,
>
> On Fri, 10 Jul 2026 07:45:28 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
>> Hi Derek,
>> 
>> On Tue, 30 Jun 2026 16:32:30 -0400
>> Derek Barbosa <debarbos@redhat.com> wrote:
>> 
>> > On Sun, Jun 07, 2026 at 07:56:56PM +0200, Mauro Carvalho Chehab wrote:  
>> > > Added both global and per-project permissions to add CI checks:
>> > > 
>> > > 	https://github.com/getpatchwork/patchwork/pull/653
>> > > 
>> > > Tested on a docker container created on the top of current
>> > > upstream, with the database imported from linuxtv.org and
>> > > migrated to the new permissions model.    
>> > 
>> > Hi Mauro,
>> > 
>> > Patchwork integration has been cleaned up and the original issues filed against
>> > the Sashiko project have been resolved.
>> > 
>> > Any progress on your work for the permissions modifications (or enabling it for
>> > media)?  
>> 
>> I'd like to have the permission modification changes applied at
>> Patchwork upstream before applying it to linux-media instance, as
>> this will require a DB change, which can make it harder in the future
>> to update linux-media patchwork instance, especially if upstream ends
>> with some changes over my proposed patch.
>> 
>> So, while patchwork doesn't have a separated permission for checks
>> addition, we'll keep using my tool to update bots feedback from
>> e-mails. To prevent too much noise at linux-media, we have now a
>> 
>> 	media-ci@linuxtv.org
>> 
>> mailing list that is meant to receive all bots feedback. The
>> e-mail parser is listening to it as well.
>
> Btw, we did an inquire among the participants of the latest media
> summit, where we discussed Sashiko.
>
> There is a consensus of receiving Sashiko feedback at 
> media-ci@linuxtv.org, where people can opt-in/opt-out.
>
> With regards to c/c authors, there was a proposal to add a custom 
> disclaimer's notice on each patch. Also, several people manifested
> that it should be possible for authors to opt-out receiving Sashiko's
> e-mails.

Sorry, I'm not sure how it's supposed to work (opt-in/opt-out).
My take is that Sashiko (the sashiko.dev instance, not private/corp
instances) is a tool for maintainers, not individual authors.
I rely on corresponding maintainers decision on whether emails are
sent to the corresponding mailing list. Individuals can set their
spam filters up if they don't want to get these emails, I can't control
it. Providing individual authors an option "I don't want my patches
to be reviewed" sound strange to me. It's like "I don't want my patches
to be tested by unit tests".

> So, at least with the current way, we were unable to reach a
> consensus (or a large majority) about c/c the author.

You can start with cc mailing list only or a set of enthusiasts.

> So, what it is a consensus is to receive Sashiko's email via
> media-ci@linuxtv.org, evaluating its feedback results for a couple 
> of kernel releases. IMO, it is better to receive there also e-mails
> if Sashiko didn't find any issue.

There is such an option, we can enable it.
Should I move on and enable reviews to be sent to media-ci@linuxtv.org?

> One interesting feedback was related to a review from i.MX DTS and DMAEngine 
> I3C patch series.
>
> I guess it could be the start of a custom linux-media prompt.
>
> The original suggestion is:
>
> 	"I suggested add some actionable advice.  The below action when I handle
> 	i.MX DTS and DMAEngine\I3C sashakio review feedback.  
>
> 	- "Pre existing issue" \ "not introduce by this patch", can omit this feedback,
> 	But encourage fix at following patch
> 	- "UAF" or lock issue,  most like is true issue, strongly suggest fix it. Maintainer
> 	Most likely double check this type issue.
> 	- Small issue like "typo" or "indention",  please fix before maintainer involve
> 	Review. 
> 	- other identify severity is HIGH issue, suggest reply sashiko email and provide
> 	Your judgement when close to land."

Re disclaimers and addition instructions in emails - we can discuss it,
but my personal preference would be to keep the minimal - after getting
like 10 emails from Sashiko it will feel like legal disclaimers written
in small font - something that nobody reads and it just wastes the
screen space. So maybe we can put these subsystem-specific rules
somewhere and just provide a link in every email? Just an idea.

> There was also a discussion about a magic number inside a loop:
>
> 	for (unsigned int i = 0; i < 56; i++) {
>
> (on such loop, "i" was used to access an array)
>
> Sashiko didn't pick this one on one of his reviews.

This is a good candidate for some generic prompt, not subsystem-specific.

>
>
> I guess we can start with a custom RAC prompt to make Sashiko
> classify its output according with a criteria similar to the
> above.
>
> E.g. maybe linux-media may start with a custom prompt similar to this
> (as part of a more complete RAC):
>
> 	### Classification categories and actions
>
> 	1. **Pre‑existing issues** - a bug or flaw that already exists in the current code and is *not introduced by this patch*.  
> 	   - **Action**: Place at the end, under "Further suggestions". Note that the issue could be addressed on a separate patch  
> 	   - Mark as: `classification: PRE_EXISTING`, `action: DEFER_FOLLOWUP`.
>
> 	2. **UAF / Lock issues** - use‑after‑free, race condition, missing lock, double unlock, deadlock, incorrect refcounting, etc.  
> 	   - **Action**: This is almost certainly a true bug. Encourage the author to check or consult an expert if in doubt.
> 	   - Mark as: `classification: CRITICAL_LOCKING_UAF`, `action: MUST_FIX`.
>
> 	3. **Small issues** - typo, indentation, coding‑style, whitespace, missing blank line, comment grammar, overly long line, etc.  
> 	   - **Action**: These should be corrected before the maintainer becomes involved in the review.  
> 	   - Mark as: `classification: STYLE_TYPO`, `action: FIX_BEFORE_MAINTAINER`.
>
> 	4. **Magic numbers** - when a numeric literal (like `56`, `128`) is used without a defined constant or obvious connection to an array size. 
> 	   - **Action**: These should be corrected before the maintainer becomes involved in the review.  
> 	   - Mark as: `classification: MAGIC_NUMBER`, `action: FIX_BEFORE_MAINTAINER`.
>
> 	5. **Other HIGH severity issues** – any problem that is clearly a functional bug (logic error, NULL deref, buffer overflow, API misuse, incorrect error handling, memory leak, etc.) that does not fall into category 2.  
> 	   - **Action**: the authors should provide their own judgment on the issue (e.g., explain why it is a false positive, or confirm the fix).  
> 	   - Mark as: `classification: HIGH_OTHER`, `action: REPLY_WITH_JUDGMENT_NEAR_LANDING`.
>
> 	6. **Low severity / Informational** – anything that does not fit the above (e.g., suggestion, minor question, potential improvement that is not a bug).  
> 	   - **Action**: Optional; can be addressed at author’s discretion.  
> 	   - Mark as: `classification: LOW_INFO`, `action: OPTIONAL`.
>
> Please notice that we're not a prompt experts - nor, afaikt, any media
> developer tested running Sashiko locally themselves - so you probably can
> come up with a better custom prompt than us.

I'll take a look, thanks!

Thank you!