| Message ID | 20240611-coccinelle-followup-v1-2-df2de9c2f320@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: Follow-up for coccinelle lock fixes patchset | expand |
Hi Hans I clicked on send too fast sorry :S CHECK: Alignment should match open parenthesis #37: FILE: drivers/media/dvb-core/dvb_frontend.c:2831: +static int wait_dvb_frontend(struct dvb_adapter *adapter, + struct dvb_device *mfedev) CHECK: Please don't use multiple blank lines #71: FILE: drivers/media/dvb-core/dvb_frontend.c:2872: + Let me know if I should resend or you can handle it while merging. I already fixed in my tree in case I have to send a v2 Thanks! On Tue, 11 Jun 2024 at 13:03, Ricardo Ribalda <ribalda@chromium.org> wrote: > > 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 > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > drivers/media/dvb-core/dvb_frontend.c | 59 ++++++++++++++++++++++------------- > 1 file changed, 38 insertions(+), 21 deletions(-) > > diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c > index e81b9996530e..a7739f5e78cb 100644 > --- a/drivers/media/dvb-core/dvb_frontend.c > +++ b/drivers/media/dvb-core/dvb_frontend.c > @@ -30,6 +30,7 @@ > #include <linux/kthread.h> > #include <linux/ktime.h> > #include <linux/compat.h> > +#include <linux/lockdep.h> > #include <asm/processor.h> > > #include <media/dvb_frontend.h> > @@ -2826,6 +2827,34 @@ static int __dvb_frontend_open(struct inode *inode, struct file *file) > return ret; > } > > +static int wait_dvb_frontend(struct dvb_adapter *adapter, > + struct dvb_device *mfedev) > +{ > + struct dvb_frontend *mfe = mfedev->priv; > + struct dvb_frontend_private *mfepriv = mfe->frontend_priv; > + int mferetry = (dvb_mfe_wait_time << 1); > + int ret = 0; > + > + lockdep_assert_held(&adapter->mfe_lock); > + > + if (mfedev->users == -1 && !mfepriv->thread) > + return 0; > + > + mutex_unlock(&adapter->mfe_lock); > + > + while (mferetry-- && (mfedev->users != -1 || mfepriv->thread)) { > + if (msleep_interruptible(500)) > + if (signal_pending(current)) { > + ret = -EINTR; > + break; > + } > + } > + > + mutex_lock(&adapter->mfe_lock); > + > + return ret; > +} > + > static int dvb_frontend_open(struct inode *inode, struct file *file) > { > struct dvb_device *dvbdev = file->private_data; > @@ -2840,19 +2869,17 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) > if (!adapter->mfe_shared) > return __dvb_frontend_open(inode, file); > > + > + guard(mutex)(&adapter->mfe_lock); > + > if (adapter->mfe_shared == 2) { > - mutex_lock(&adapter->mfe_lock); > if ((file->f_flags & O_ACCMODE) != O_RDONLY) { > if (adapter->mfe_dvbdev && > - !adapter->mfe_dvbdev->writers) { > - mutex_unlock(&adapter->mfe_lock); > + !adapter->mfe_dvbdev->writers) > return -EBUSY; > - } > adapter->mfe_dvbdev = dvbdev; > } > } else { > - mutex_lock(&adapter->mfe_lock); > - > if (!adapter->mfe_dvbdev) { > adapter->mfe_dvbdev = dvbdev; > } else if (adapter->mfe_dvbdev != dvbdev) { > @@ -2862,34 +2889,24 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) > *mfe = mfedev->priv; > struct dvb_frontend_private > *mfepriv = mfe->frontend_priv; > - int mferetry = (dvb_mfe_wait_time << 1); > - > - mutex_unlock(&adapter->mfe_lock); > - while (mferetry-- && (mfedev->users != -1 || > - mfepriv->thread)) { > - if (msleep_interruptible(500)) { > - if (signal_pending(current)) > - return -EINTR; > - } > - } > > - mutex_lock(&adapter->mfe_lock); > + ret = wait_dvb_frontend(adapter, mfedev); > + if (ret) > + return ret; > + > if (adapter->mfe_dvbdev != dvbdev) { > mfedev = adapter->mfe_dvbdev; > mfe = mfedev->priv; > mfepriv = mfe->frontend_priv; > if (mfedev->users != -1 || > - mfepriv->thread) { > - mutex_unlock(&adapter->mfe_lock); > + mfepriv->thread) > return -EBUSY; > - } > adapter->mfe_dvbdev = dvbdev; > } > } > } > > ret = __dvb_frontend_open(inode, file); > - mutex_unlock(&adapter->mfe_lock); > > return ret; > } > > -- > 2.45.2.505.gda0bf45e8d-goog >
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c index e81b9996530e..a7739f5e78cb 100644 --- a/drivers/media/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb-core/dvb_frontend.c @@ -30,6 +30,7 @@ #include <linux/kthread.h> #include <linux/ktime.h> #include <linux/compat.h> +#include <linux/lockdep.h> #include <asm/processor.h> #include <media/dvb_frontend.h> @@ -2826,6 +2827,34 @@ static int __dvb_frontend_open(struct inode *inode, struct file *file) return ret; } +static int wait_dvb_frontend(struct dvb_adapter *adapter, + struct dvb_device *mfedev) +{ + struct dvb_frontend *mfe = mfedev->priv; + struct dvb_frontend_private *mfepriv = mfe->frontend_priv; + int mferetry = (dvb_mfe_wait_time << 1); + int ret = 0; + + lockdep_assert_held(&adapter->mfe_lock); + + if (mfedev->users == -1 && !mfepriv->thread) + return 0; + + mutex_unlock(&adapter->mfe_lock); + + while (mferetry-- && (mfedev->users != -1 || mfepriv->thread)) { + if (msleep_interruptible(500)) + if (signal_pending(current)) { + ret = -EINTR; + break; + } + } + + mutex_lock(&adapter->mfe_lock); + + return ret; +} + static int dvb_frontend_open(struct inode *inode, struct file *file) { struct dvb_device *dvbdev = file->private_data; @@ -2840,19 +2869,17 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) if (!adapter->mfe_shared) return __dvb_frontend_open(inode, file); + + guard(mutex)(&adapter->mfe_lock); + if (adapter->mfe_shared == 2) { - mutex_lock(&adapter->mfe_lock); if ((file->f_flags & O_ACCMODE) != O_RDONLY) { if (adapter->mfe_dvbdev && - !adapter->mfe_dvbdev->writers) { - mutex_unlock(&adapter->mfe_lock); + !adapter->mfe_dvbdev->writers) return -EBUSY; - } adapter->mfe_dvbdev = dvbdev; } } else { - mutex_lock(&adapter->mfe_lock); - if (!adapter->mfe_dvbdev) { adapter->mfe_dvbdev = dvbdev; } else if (adapter->mfe_dvbdev != dvbdev) { @@ -2862,34 +2889,24 @@ static int dvb_frontend_open(struct inode *inode, struct file *file) *mfe = mfedev->priv; struct dvb_frontend_private *mfepriv = mfe->frontend_priv; - int mferetry = (dvb_mfe_wait_time << 1); - - mutex_unlock(&adapter->mfe_lock); - while (mferetry-- && (mfedev->users != -1 || - mfepriv->thread)) { - if (msleep_interruptible(500)) { - if (signal_pending(current)) - return -EINTR; - } - } - mutex_lock(&adapter->mfe_lock); + ret = wait_dvb_frontend(adapter, mfedev); + if (ret) + return ret; + if (adapter->mfe_dvbdev != dvbdev) { mfedev = adapter->mfe_dvbdev; mfe = mfedev->priv; mfepriv = mfe->frontend_priv; if (mfedev->users != -1 || - mfepriv->thread) { - mutex_unlock(&adapter->mfe_lock); + mfepriv->thread) return -EBUSY; - } adapter->mfe_dvbdev = dvbdev; } } } ret = __dvb_frontend_open(inode, file); - mutex_unlock(&adapter->mfe_lock); return ret; }
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 Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- drivers/media/dvb-core/dvb_frontend.c | 59 ++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 21 deletions(-)