Linux Media Controller development
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: Roman Gushchin <roman.gushchin@linux.dev>,
	Derek Barbosa <debarbos@redhat.com>,
	Matthieu Baerts <matttbe@kernel.org>,
	Konstantin Ryabitsev <konstantin@linuxfoundation.org>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Steven Rostedt <rostedt@goodmis.org>,
	users@kernel.org,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	Stephen Finucane <stephenfin@redhat.com>
Subject: Re: Linking Patchwork with Sashiko?
Date: Mon, 13 Jul 2026 12:41:20 +0300	[thread overview]
Message-ID: <20260713094120.GD1127719@killaraus.ideasonboard.com> (raw)
In-Reply-To: <20260713095538.3d5e86f1@foz.lan>

On Mon, Jul 13, 2026 at 09:55:38AM +0200, Mauro Carvalho Chehab wrote:
> On Fri, 10 Jul 2026 18:01:38 -0700 Roman Gushchin wrote:
> > Mauro Carvalho Chehab writes:
> > > On Fri, 10 Jul 2026 07:45:28 +0200 Mauro Carvalho Chehab wrote:
> > >> On Tue, 30 Jun 2026 16:32:30 -0400 Derek Barbosa 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.
> 
> IMO, sashiko.dev could have an e-mail interface to enable/disable
> delivers to authors on a similar way to what mailman does. For instance, 
> if an author replies to be "unsubscribed" (or access some web interface),
> a confirm e-mail with an unique token ID would be sent to his e-mail.
> After replying to it, the author would be blacklisted on e-mail
> delivery.
> 
> This is for authors only.
> 
> > I rely on corresponding maintainers decision on whether emails are
> > sent to the corresponding mailing list. 
> 
> Mailing lists is a different case: the decision should be taken by
> maintainers. Only maintainers can change that.
> 
> > 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".
> 
> I agree with you, and, on my head, not sending e-mails to the author
> is a clear violation to one of the most basic net etiquette rule on
> mailing lists: any replies to posts there should reach the author.

I don't know where that one comes from.

What happened to this other "most basic rule" that subscription to
services that deliver e-mails should be opt-in ?

> Unfortunately, some people have too strong opinions against LLM,
> and don't even want to setup filters related to it.
> 
> > > 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.
> 
> Yes.
> 
> > > 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?
> 
> Yes, please. This way, media developers interested on receiving Sashiko
> reviews may get access to it. It will also feed media patchwork instance
> at:
> 	https://patchwork.linuxtv.org/project/linux-media/list/
> 
> > > 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.
> 
> Disclaimers on e-mails are, IMO, important to new contributors. You're
> right that experienced developers will ignore it, but having a simple
> disclaimer's notice informing that bot reviews may make mistakes and
> that, in case of doubt one must consult an experienced developer is a
> good thing. We had some discussions on media to have a prompt similar
> to this:
> 
>     This is an LLM-generated review that can be prone to hallucinations.
>     It is fine to ignore this review.
>     When in doubt about some of the findings, please consult
>     an experienced developer.
> 
> I even tried wrote some code adding it at:
> 	https://github.com/mchehab/sashiko/commit/5e3ec00772e9bc7a9e7d90993674d822a7bc200f
> 	https://github.com/mchehab/sashiko/commit/eff8622fe2828efc56cd38037cb837d28ebd3914
> 
> they are on this branch:
> 	https://github.com/mchehab/sashiko/tree/linux-media
> 
> but this was before the discussions with regards to c/c authors.
> Also, please notice that I never wrote Rust code before, so the
> rust patch is likely not perfect: it is compile-tested only.
> 
> > 
> > > 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.
> 
> Agreed.
> 
> > > 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!
> 
> Btw, it occurred to me that we could also pick some things from Media Subsystem 
> profile as well, from its addendum:
> 	https://docs.kernel.org/driver-api/media/maintainer-entry-profile.html#submit-checklist-addendum
> 
> like adding a feedback that new drivers need to pass at the compliance
> tools if the patch series is adding new drivers.

-- 
Regards,

Laurent Pinchart

      reply	other threads:[~2026-07-13  9:41 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260528144627.1ae09ff2@foz.lan>
     [not found] ` <1372F826-5513-4EB2-AE27-1DC0D2DE0AEB@linux.dev>
     [not found]   ` <20260529083100.6710b6cd@foz.lan>
     [not found]     ` <20260529083801.2c7e8990@foz.lan>
     [not found]       ` <ahmwUk0uXTkdwohf@debarbos-thinkpadt14gen5.rmtusma.csb>
2026-05-30  8:30         ` Linking Patchwork with Sashiko? Mauro Carvalho Chehab
2026-05-30 15:57           ` Roman Gushchin
2026-05-30 18:00             ` Mauro Carvalho Chehab
2026-05-30 18:49               ` Mauro Carvalho Chehab
2026-05-30 18:53                 ` Mauro Carvalho Chehab
2026-06-02 15:51                   ` Derek Barbosa
2026-06-02 16:51                     ` Mauro Carvalho Chehab
2026-06-02 18:39                       ` Jason Gunthorpe
2026-06-02 20:29                         ` Mauro Carvalho Chehab
2026-06-02 20:13                     ` Roman Gushchin
2026-06-02 20:39                       ` Mauro Carvalho Chehab
2026-06-02 20:44                         ` Roman Gushchin
2026-06-02 23:50                         ` Matthieu Baerts
2026-06-03  3:35                           ` Mauro Carvalho Chehab
2026-06-03  3:49                             ` Roman Gushchin
2026-06-04  6:52                           ` Mauro Carvalho Chehab
2026-06-07 17:56                             ` Mauro Carvalho Chehab
2026-06-30 20:32                               ` Derek Barbosa
2026-07-10  5:45                                 ` Mauro Carvalho Chehab
2026-07-10  6:39                                   ` Mauro Carvalho Chehab
2026-07-11  1:01                                     ` Roman Gushchin
2026-07-13  7:55                                       ` Mauro Carvalho Chehab
2026-07-13  9:41                                         ` Laurent Pinchart [this message]

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=20260713094120.GD1127719@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=debarbos@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=konstantin@linuxfoundation.org \
    --cc=linux-media@vger.kernel.org \
    --cc=matttbe@kernel.org \
    --cc=mchehab+huawei@kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=stephenfin@redhat.com \
    --cc=users@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox