Re: [PATCH 12/44] metag: Use get_signal() signal_setup_done()

Re: [PATCH 12/44] metag: Use get_signal() signal_setup_done()

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2764 bytes --]

Hi Richard,

On 02/03/14 23:57, Richard Weinberger wrote:
> @@ -235,10 +230,8 @@ static void handle_signal(unsigned long sig, siginfo_t *info,
>  static int do_signal(struct pt_regs *regs, int syscall)
>  {
>  	unsigned int retval = 0, continue_addr = 0, restart_addr = 0;
> -	struct k_sigaction ka;
> -	siginfo_t info;
> -	int signr;
>  	int restart = 0;
> +	struct ksignal ksig;
>  
>  	/*
>  	 * By the end of rt_sigreturn the context describes the point that the
> @@ -272,30 +265,30 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	}
>  
>  	/*
> -	 * Get the signal to deliver. When running under ptrace, at this point
> -	 * the debugger may change all our registers ...
> -	 */
> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
> -	/*
>  	 * Depending on the signal settings we may need to revert the decision
>  	 * to restart the system call. But skip this if a debugger has chosen to
>  	 * restart at a different PC.
>  	 */
>  	if (regs->REG_PC != restart_addr)
>  		restart = 0;
> -	if (signr > 0) {
> +
> +	/*
> +	 * Get the signal to deliver. When running under ptrace, at this point
> +	 * the debugger may change all our registers ...
> +	 */
> +	if (get_signal(&ksig)) {

The patch all looks good to me except for this bit. You've moved the bit
at which a ptrace debugger can alter the registers to after the check
for whether the PC has been altered, which would still need to cancel
the restart.

Does something like the hunks below look reasonable to you?

Would you like me to take this patch through the metag tree or do you
want to keep them together?

Cheers
James

@@ -275,7 +268,7 @@ static int do_signal(struct pt_regs *regs, int syscall)
 	 * Get the signal to deliver. When running under ptrace, at this point
 	 * the debugger may change all our registers ...
 	 */
-	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
+	get_signal(&ksig);
 	/*
 	 * Depending on the signal settings we may need to revert the decision
 	 * to restart the system call. But skip this if a debugger has chosen to
@@ -283,19 +276,19 @@ static int do_signal(struct pt_regs *regs, int syscall)
 	 */
 	if (regs->REG_PC != restart_addr)
 		restart = 0;
-	if (signr > 0) {
+	if (ksig.sig > 0) {
 		if (unlikely(restart)) {
 			if (retval == -ERESTARTNOHAND
 			    || retval == -ERESTART_RESTARTBLOCK
 			    || (retval == -ERESTARTSYS
-				&& !(ka.sa.sa_flags & SA_RESTART))) {
+				&& !(ksig.ka.sa.sa_flags & SA_RESTART))) {
 				regs->REG_RETVAL = -EINTR;
 				regs->REG_PC = continue_addr;
 			}
 		}
 
 		/* Whee! Actually deliver the signal.  */
-		handle_signal(signr, &info, &ka, regs);
+		handle_signal(&ksig, regs);
 		return 0;
 	}
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 12/44] metag: Use get_signal() signal_setup_done()

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

On 03/03/14 11:15, James Hogan wrote:
> Hi Richard,
> 
> On 02/03/14 23:57, Richard Weinberger wrote:
>> @@ -235,10 +230,8 @@ static void handle_signal(unsigned long sig, siginfo_t *info,
>>  static int do_signal(struct pt_regs *regs, int syscall)
>>  {
>>  	unsigned int retval = 0, continue_addr = 0, restart_addr = 0;
>> -	struct k_sigaction ka;
>> -	siginfo_t info;
>> -	int signr;
>>  	int restart = 0;
>> +	struct ksignal ksig;
>>  
>>  	/*
>>  	 * By the end of rt_sigreturn the context describes the point that the
>> @@ -272,30 +265,30 @@ static int do_signal(struct pt_regs *regs, int syscall)
>>  	}
>>  
>>  	/*
>> -	 * Get the signal to deliver. When running under ptrace, at this point
>> -	 * the debugger may change all our registers ...
>> -	 */
>> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
>> -	/*
>>  	 * Depending on the signal settings we may need to revert the decision
>>  	 * to restart the system call. But skip this if a debugger has chosen to
>>  	 * restart at a different PC.
>>  	 */
>>  	if (regs->REG_PC != restart_addr)
>>  		restart = 0;
>> -	if (signr > 0) {
>> +
>> +	/*
>> +	 * Get the signal to deliver. When running under ptrace, at this point
>> +	 * the debugger may change all our registers ...
>> +	 */
>> +	if (get_signal(&ksig)) {
> 
> The patch all looks good to me except for this bit. You've moved the bit
> at which a ptrace debugger can alter the registers to after the check
> for whether the PC has been altered, which would still need to cancel
> the restart.
> 
> Does something like the hunks below look reasonable to you?
> 
> Would you like me to take this patch through the metag tree or do you
> want to keep them together?

I've applied the altered version of this patch to the metag tree for v3.15.

Thanks
James

> 
> Cheers
> James
> 
> @@ -275,7 +268,7 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	 * Get the signal to deliver. When running under ptrace, at this point
>  	 * the debugger may change all our registers ...
>  	 */
> -	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
> +	get_signal(&ksig);
>  	/*
>  	 * Depending on the signal settings we may need to revert the decision
>  	 * to restart the system call. But skip this if a debugger has chosen to
> @@ -283,19 +276,19 @@ static int do_signal(struct pt_regs *regs, int syscall)
>  	 */
>  	if (regs->REG_PC != restart_addr)
>  		restart = 0;
> -	if (signr > 0) {
> +	if (ksig.sig > 0) {
>  		if (unlikely(restart)) {
>  			if (retval == -ERESTARTNOHAND
>  			    || retval == -ERESTART_RESTARTBLOCK
>  			    || (retval == -ERESTARTSYS
> -				&& !(ka.sa.sa_flags & SA_RESTART))) {
> +				&& !(ksig.ka.sa.sa_flags & SA_RESTART))) {
>  				regs->REG_RETVAL = -EINTR;
>  				regs->REG_PC = continue_addr;
>  			}
>  		}
>  
>  		/* Whee! Actually deliver the signal.  */
> -		handle_signal(signr, &info, &ka, regs);
> +		handle_signal(&ksig, regs);
>  		return 0;
>  	}
>  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]