Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 5/8] powerpc/watchpoint: Fix exception handling for
 CONFIG_HAVE_HW_BREAKPOINT=N
To:     Ravi Bangoria <ravi.bangoria@linux.ibm.com>,
        christophe.leroy@c-s.fr
Cc:     mpe@ellerman.id.au, mikey@neuling.org, paulus@samba.org,
        naveen.n.rao@linux.vnet.ibm.com, pedromfc@linux.ibm.com,
        rogealve@linux.ibm.com, jniethe5@gmail.com,
        linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
References: <20200825043617.1073634-1-ravi.bangoria@linux.ibm.com>
 <20200825043617.1073634-6-ravi.bangoria@linux.ibm.com>
 <91d34b89-603a-fddc-ea0f-53a79b287eed@csgroup.eu>
 <1e706137-3de9-a713-5a86-48fbc5e6f740@linux.ibm.com>
From:   Christophe Leroy <christophe.leroy@csgroup.eu>
Message-ID: <aeff0680-d260-26ab-9f32-ac6ac1df2df3@csgroup.eu>
Date:   Tue, 25 Aug 2020 14:06:21 +0200
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.11.0
MIME-Version: 1.0
In-Reply-To: <1e706137-3de9-a713-5a86-48fbc5e6f740@linux.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: fr
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Le 25/08/2020 à 13:07, Ravi Bangoria a écrit :
> Hi Christophe,
> 
>>> diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c 
>>> b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
>>> index 57a0ab822334..866597b407bc 100644
>>> --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
>>> +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
>>> @@ -286,11 +286,16 @@ long ppc_del_hwdebug(struct task_struct *child, 
>>> long data)
>>>       }
>>>       return ret;
>>>   #else /* CONFIG_HAVE_HW_BREAKPOINT */
>>> +    if (child->thread.hw_brk[data - 1].flags & HW_BRK_FLAG_DISABLED)
>>
>> I think child->thread.hw_brk[data - 1].flags & HW_BRK_FLAG_DISABLED 
>> should go around additionnal ()
> 
> Not sure I follow.

Neither do I ....

I thought that GCC would emit a warning for that, but in fact it only 
emit warnings for things like:

	if (flags & HW_BRK_FLAG_DISABLED == HW_BRK_FLAG_DISABLED)

> 
>>
>>> +        goto del;
>>> +
>>>       if (child->thread.hw_brk[data - 1].address == 0)
>>>           return -ENOENT;
>>
>> What about replacing the above if by:
>>      if (!(child->thread.hw_brk[data - 1].flags) & 
>> HW_BRK_FLAG_DISABLED) &&
>>          child->thread.hw_brk[data - 1].address == 0)
>>          return -ENOENT;
> okay.. that's more compact.
> 
> But more importantly, what I wanted to know is whether 
> CONFIG_HAVE_HW_BREAKPOINT
> is set or not in production/distro builds for 8xx. Because I see it's 
> not set in
> 8xx defconfigs.

Yes in our production configs with have CONFIG_PERF_EVENTS, that implies 
CONFIG_HAVE_HW_BREAKPOINT

Christophe