On 04/24/2013 03:48 PM, Frederic Weisbecker wrote: > 2013/4/24 Oleg Nesterov <oleg@xxxxxxxxxx>: >> DR6_RESERVED and DR_CONTROL_RESERVED are used to clear the set >> bits in the "unsigned long" data, make them long to ensure that >> "&~" doesn't clear the upper bits. >> >> do_debug() and ptrace_write_dr7() which use DR*_RESERVED look >> safe, but probably it makes sense to cleanup anyway. > > Agreed. The code looks safe, but the pattern is error prone. I'm all > for that cleanup. > Just something below: > >> >> Reported-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> >> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx> >> --- >> arch/x86/include/uapi/asm/debugreg.h | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/uapi/asm/debugreg.h >> index 3c0874d..75d07dd 100644 >> --- a/arch/x86/include/uapi/asm/debugreg.h >> +++ b/arch/x86/include/uapi/asm/debugreg.h >> @@ -15,7 +15,7 @@ >> are either reserved or not of interest to us. */ >> >> /* Define reserved bits in DR6 which are always set to 1 */ >> -#define DR6_RESERVED (0xFFFF0FF0) >> +#define DR6_RESERVED (0xFFFF0FF0ul) > > You told in an earlier email that intel manual says upper 32 bits of > dr6 are reserved. > In this case don't we need to expand the mask in 64 bits like is done > for DR_CONTROL_RESERVED? > Arguably this would be a *good* use for ~ ... Instead of defining separate bitmasks for 32 and 64 bits have the reciprocal (non-reserved bits): #define DR6_RESERVED (~0x0000F00FUL) That does have the right value on both 32 and 64 bits. The leading zeroes aren't even really needed. Now, DR6 is a bit special in that a bunch of the reserved bits are hardwired to 1, not 0; I don't know offhand if that is true for bits [63:32]. -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
