RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Thank you,
Best regards,
Hari

> -----Original Message-----
> From: Marathe, Yogesh
> Sent: Friday, July 30, 2010 1:53 AM
> To: Kanigeri, Hari; Premi, Sanjeev; Linux Omap; Tony Lindgren
> Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> Subject: RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
> 
> Hari,
> 
> > -----Original Message-----
> > From: Kanigeri, Hari
> > Sent: Thursday, July 29, 2010 6:44 PM
> > To: Marathe, Yogesh; Premi, Sanjeev; Linux Omap; Tony Lindgren
> > Cc: Shilimkar, Santosh; Cousson, Benoit; Que, Simon
> > Subject: RE: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver
> >
> > Yogesh,
> >
> > Nice to see your email.
> >
> > > > > +/* Release a spinlock */
> > > > > +int hwspinlock_unlock(struct hwspinlock *handle)
> > > > > +{
> > > > > +     if (WARN_ON(handle == NULL))
> > > > > +             return -EINVAL;
> > > > > +
> > > > > +     /* Release it by writing 0 to it */
> > > > > +     writel(0, handle->lock_reg);
> > >
> > > [[Yogesh Marathe]] Releasing the spinlock without knowing who owns it
> is
> > > risky. There should be a check for ownership and if authenticated user
> has
> > > called this api  only then it should be released otherwise permission
> > > denied error should be returned.
> >
> > -- I think if there is another Kernel client that is trying to release
> that is not owned by
> > it then that Kernel client itself is buggy and needs to be fixed. Please
> share your
> > thoughts on how we can ensure that we can add some protection.
> 
> [Yogesh Marathe] I agree the client has to be fixed but at least the
> client should get some error message instead of crashing someone else. I
> think if hwspinlock_request() and hwspinlock_request_specific() can keep
> the owner info (say thread id) in some module state structure it is
> possible to verify against it before acquiring and releasing the lock

-- I agree with Benoit's point. We don't have to complicate this basic IP block by adding additional checks. 
I2C Kernel module is one of the client and I am not sure if you can rely on thread id for ownership. 

> 
> >
> >
> > >
> > > > > +
> > > > > +     pm_runtime_put(&handle->pdev->dev);
> > > > > +
> > > > > +     return 0;
> >
> > Thank you,
> > Best regards,
> > Hari
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux