Re: [KJ][PATCH]Rocket port:use mutex instead of binary semaphore

[Milind Arun Choudhary]

On 10:46 Mon 16 Apr     , Roland Dreier wrote:
>  > -	down_interruptible(&info->write_sem);
>  > +	if(mutex_lock_interruptible(&info->write_lock)){
>  > +		return -ERESTARTSYS;
>  > +	}
> 
> 1) This is a semantic change.  Of course using down_interruptible()
> without checking the return value is almost certainly a bug, but have
> you thought about whether returning ERESTARTSYS is correct here?  If
> you have, then please include the reasoning in your patch description.
> (Another possibility would be to just use an uninterruptible mutex_lock())
no,frankly not much..
and something which i didn't understand is
why a down  in one control path & an down_interruptible in other?

but i had in mind,which you rightly pointed out 
handling the return from _interruptible.

i have some bits and pieces of information regarding thie,
searching for more, and correction to my current understanding,
  so that i can make something meaningfull out of it.

earlier kernels used to return in EINTR & it was up to the use space to 
handle the interrupted system call. ..reissue..


then ERESTARTSYS mechanism was added and the kernel could restart the 
syscall on its own

so its up to us whether we want the interruption to be taken care by 
the userspace or kernel.
i.e return EINTR or ERESTARTSYS respectively.
CMIIW.


also i read somewhere that
ERESTARTSYS should never be seen by user space code.  The kernel should
either restart the current syscall or convert the code to EINTR before
returning to user space.  If you are seeing ERESTARTSYS in user space
then it is a kernel bug.


> 2) The coding style for the if statement is not quite right.  The
> correct way is to do
> 
> 	if (condition)
> 		one_liner;
> 
> (note the space between the 'if' and the '(', and no braces used)
> 
ok.. got it..

-- 
Milind Arun Choudhary
-
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/