Messages in this thread | | | Date | Fri, 23 May 2008 11:57:41 +0100 | From | Alan Cox <> | Subject | Re: [PATCH] raw1394: Push the BKL down into the driver ioctls |
| |
On Fri, 23 May 2008 12:23:09 +0200 Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Alan Cox wrote: > > Actually in this case wrap the function for now. > > > > Signed-off-by: Alan Cox <alan@redhat.com> > > Can an .unlocked_ioctl() preempt another .unlocked_ioctl() to the very > same instance of struct file?
Yes. And this btw is true even with the old locked ioctl call if you ever sleep (eg a copy_to/from_user).
> If yes, we need to serialize do_raw1394_ioctl against itself or come up > with another protection against concurrent fi->iso_state switches before > we can remove lock_kernel(). And if a .write() can preempt another > .write() to the same instance of struct file, raw1394_write() already > has a problem with concurrent fi->state switches.
Quite a few drivers end up with a private mutex and do mutex_lock/unlock around the ioctl and write paths (and if the write path can be slow using _trylock and O_NDELAY check when appropriate).
The goal of pushing it down is to enable driver authors to see and to do the locking at a driver level instead - plus fix lots of bugs where the BKL "sleep and drop" behaviour isn't anticipated.
> The same s/raw1394_ioctl/do_raw1394_ioctl/ should be done in > raw1394_compat_ioctl(). But I suppose it doesn't really matter because > lock_kernel() is allowed to nest.
Agreed.
Alan
| |