Re: [PATCH] Xilinx: hwicap: cleanup

[Jiri Slaby]

On 02/25/2008 12:21 AM, Stephen Neuendorffer wrote:
> > >  @@ -549,8 +556,7 @@ static int hwicap_release(struct inode *inode,
> struct file *file)
> > >         int i;
> > >         int status = 0;
> > >
> > >  -       if (down_interruptible(&drvdata->sem))
> > >  -               return -ERESTARTSYS;
> > >  +       mutex_lock(&drvdata->sem);
> > Why not mutex_lock_interruptible()? (goes for all cases of
> mutex_lock())
>
> It's not clear to me how to get 'correct' behavior in these functions if
> the interrupt happens.  For instance in probe/setup, if the mutex_lock
> is interrupted, it doesn't appear that there is anything to do other
> than return an error code that no device is present?  I think this was
> suggested by Jiri...

Yeah, since ERESTARTSYS would be ignored from f_op->release (see __fput()), 
drv->probe (see really_probe() and probe_failed label); not even talking about 
void return value functions. In those cases, the device won't be de/initialized 
and might result in unwanted behaviour (multiple modprobes for one device, 
rmmod/insmod need if you hit the path in release etc.).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/