All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Ricardo Ribalda <ribalda@chromium.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>
Subject: Re: [PATCH v2 2/2] media: drivers/media/dvb-core: Refactor dvb_frontend_open locking
Date: Fri, 16 Aug 2024 12:16:57 +0200	[thread overview]
Message-ID: <20240816121657.69898770@foz.lan> (raw)
In-Reply-To: <CANiDSCtk=_Qb4aC15otZiUrdysV2h82ihbA2eP2kWyQSovq=MQ@mail.gmail.com>

Em Fri, 16 Aug 2024 10:20:47 +0200
Ricardo Ribalda <ribalda@chromium.org> escreveu:

> Hi Mauro
> 
> On Fri, 16 Aug 2024 at 10:17, Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Em Wed, 14 Aug 2024 14:10:23 +0000
> > Ricardo Ribalda <ribalda@chromium.org> escreveu:
> >  
> > > Split out the wait function, and introduce some new toys: guard and
> > > lockdep.
> > >
> > > This fixes the following cocci warnings:
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
> > > drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809  
> >
> > Hi Ricardo,  
> 
> Hi Mauro
> 
> >
> > Every time someone tries to fix this lock, we end having regression reports,
> > because of the diversity of devices, and the way they registers there.
> >
> > That's specially true for devices with multiple frontends and custom
> > zigzag methods.
> >
> > On what devices have you tested this patch?  
> 
> I do not have access to any device, it is just "compiled tested".
> 
> I think that the patch is mainly a refactor, it does not really change
> how the lock is handled.

While I liked your approach, in the specific case of this lock, I have 
to disagree: there were at least 2 or 3 previous attempts to fix 
issues on it, with patches made by someone and dully tested on some
hardware, ended being reverted due to corner cases with some boards.

So, I'm not willing to take the risk of accept patches touching
dvb frontend locking schema without tests with real hardware covering
common and corner cases (multi-frontend, custom zigzag, ...) and/or 
a formal code verification to proof that the lock works on all 
circumstances.

Regards,
Mauro

      reply	other threads:[~2024-08-16 10:17 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-14 14:10 [PATCH v2 0/2] media: Follow-up for coccinelle lock fixes patchset Ricardo Ribalda
2024-08-14 14:10 ` [PATCH v2 1/2] media: drivers/media/dvb-core: Split dvb_frontend_open() Ricardo Ribalda
2024-08-16 12:30   ` Ricardo Ribalda
2024-08-14 14:10 ` [PATCH v2 2/2] media: drivers/media/dvb-core: Refactor dvb_frontend_open locking Ricardo Ribalda
2024-08-16  8:16   ` Mauro Carvalho Chehab
2024-08-16  8:20     ` Ricardo Ribalda
2024-08-16 10:16       ` Mauro Carvalho Chehab [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=20240816121657.69898770@foz.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=ribalda@chromium.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 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.