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 v3 13/16] powerpc/watchpoint: Prepare handler to handle
 more than one watcnhpoint
To:     Christophe Leroy <christophe.leroy@c-s.fr>
Cc:     mpe@ellerman.id.au, mikey@neuling.org, apopple@linux.ibm.com,
        paulus@samba.org, npiggin@gmail.com,
        naveen.n.rao@linux.vnet.ibm.com, peterz@infradead.org,
        jolsa@kernel.org, oleg@redhat.com, fweisbec@gmail.com,
        mingo@kernel.org, linuxppc-dev@lists.ozlabs.org,
        linux-kernel@vger.kernel.org,
        Ravi Bangoria <ravi.bangoria@linux.ibm.com>
References: <20200414031659.58875-1-ravi.bangoria@linux.ibm.com>
 <20200414031659.58875-14-ravi.bangoria@linux.ibm.com>
 <67349339-03df-091a-47f0-78c1b14c5981@c-s.fr>
From:   Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Message-ID: <f571127c-89b0-a333-be71-18f411dc6586@linux.ibm.com>
Date:   Fri, 24 Apr 2020 09:02:12 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <67349339-03df-091a-47f0-78c1b14c5981@c-s.fr>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Christophe,

>> @@ -101,14 +129,20 @@ static bool is_ptrace_bp(struct perf_event *bp)
>>    */
>>   void arch_unregister_hw_breakpoint(struct perf_event *bp)
>>   {
>> +    int i;
>> +
> 
> This declaration should be in the block using it.
> 
>>       /*
>>        * If the breakpoint is unregistered between a hw_breakpoint_handler()
>>        * and the single_step_dabr_instruction(), then cleanup the breakpoint
>>        * restoration variables to prevent dangling pointers.
>>        * FIXME, this should not be using bp->ctx at all! Sayeth peterz.
>>        */
>> -    if (bp->ctx && bp->ctx->task && bp->ctx->task != ((void *)-1L))
>> -        bp->ctx->task->thread.last_hit_ubp = NULL;
>> +    if (bp->ctx && bp->ctx->task && bp->ctx->task != ((void *)-1L)) {
> 
> Add declaration of 'int i' here.

How will that help? Keeping declaration at the start of function is also
common practice and I don't see any recommendation to move them inside
conditional block.

Thanks,
Ravi