Re: [PATCH 3/3] Fix copy_user on x86_64

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Vitaly Mayatskikh
Date: Monday, June 30, 2008 - 8:12 am

Linus Torvalds <torvalds@linux-foundation.org> writes:


Sorry... But what was mentioned in Documentation/SubmittingPatches with:

"For this reason, all patches should be submitting e-mail "inline".
WARNING:  Be wary of your editor's word-wrap corrupting your patch,
if you choose to cut-n-paste your patch."

My first thought was "should be attached inline".


Agreed. Code was reworked again, will test it a bit more. Two more
questions to you and Andi:

1. Do you see any reasons to do fix alignment for destination as it was
done in copy_user_generic_unrolled (yes, I know, access to unaligned
address is slower)? It tries to byte-copy unaligned bytes first and then
to do a normal copy. I think, most times destination addresses will be
aligned and this check is not so necessary. If it is necessary, then
copy_user_generic_string should do the same.

2. What is the purpose of "minor optimization" in commit
3022d734a54cbd2b65eea9a024564821101b4a9a?

ENTRY(copy_user_generic_string)
        CFI_STARTPROC
        movl %ecx,%r8d          /* save zero flag */
        movl %edx,%ecx
        shrl $3,%ecx
        andl $7,%edx
        jz   10f
1:      rep
        movsq
        movl %edx,%ecx
2:      rep
        movsb
9:      movl %ecx,%eax
        ret
        
        /* multiple of 8 byte */
10:     rep
        movsq
        xor %eax,%eax
        ret

I don't think CPU is able to speculate with 'rep movs*' in both
branches, and I'm not sure if conditional jump is cheaper then empty
'rep movsb' (when ecx is 0). I want to eliminate it if you don't have
any objections.

Thanks.
-- 
wbr, Vitaly
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH 3/3] Fix copy_user on x86_64, Vitaly Mayatskikh, (Fri Jun 27, 2:52 pm)
Re: [PATCH 3/3] Fix copy_user on x86_64, Linus Torvalds, (Sat Jun 28, 11:26 am)
Re: [PATCH 3/3] Fix copy_user on x86_64, Vitaly Mayatskikh, (Mon Jun 30, 8:12 am)
Re: [PATCH 3/3] Fix copy_user on x86_64, Linus Torvalds, (Mon Jun 30, 8:55 am)
Re: [PATCH 3/3] Fix copy_user on x86_64, Andi Kleen, (Mon Jun 30, 9:16 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Wed Jul 2, 6:48 am)
Re: [PATCH 2/2] Fix copy_user on x86, Vitaly Mayatskikh, (Wed Jul 2, 6:53 am)
Re: [PATCH 2/2] Fix copy_user on x86, Andi Kleen, (Wed Jul 2, 7:08 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Wed Jul 2, 7:31 am)
Re: [PATCH 2/2] Fix copy_user on x86, Vitaly Mayatskikh, (Wed Jul 2, 7:36 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Wed Jul 2, 8:32 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Wed Jul 2, 8:58 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Linus Torvalds, (Wed Jul 2, 7:35 pm)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Mon Jul 7, 5:09 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Mon Jul 7, 5:12 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Linus Torvalds, (Mon Jul 7, 9:21 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Mon Jul 7, 10:05 am)
Re: [PATCH 1/2] Introduce copy_user_handle_tail routine, Vitaly Mayatskikh, (Wed Jul 9, 6:16 am)