Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Mon, 18 Apr 2022 15:44:18 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
Cc:     Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Ingo Molnar <mingo@redhat.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Paul Mackerras <paulus@samba.org>,
        linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v1 07/22] powerpc/ftrace: Use patch_instruction() return
 directly
Message-ID: <20220418154418.72ccc00c@gandalf.local.home>
In-Reply-To: <1650262058.altfknhzto.naveen@linux.ibm.com>
References: <cover.1648131740.git.christophe.leroy@csgroup.eu>
        <b4505e936e1aee411f7132a27791cf138102f35f.1648131740.git.christophe.leroy@csgroup.eu>
        <1650262058.altfknhzto.naveen@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 18 Apr 2022 11:51:16 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> > --- a/arch/powerpc/kernel/trace/ftrace.c
> > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > @@ -78,10 +78,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
> >  	}
> > 
> >  	/* replace the text with the new text */
> > -	if (patch_instruction((u32 *)ip, new))
> > -		return -EPERM;
> > -
> > -	return 0;
> > +	return patch_instruction((u32 *)ip, new);  
> 
> I think the reason we were returning -EPERM is so that ftrace_bug() can 

That is correct.

> throw the right error message. That will change due to this patch, 
> though I'm not sure how much it matters. -EFAULT and -EPERM seem to 
> print almost the same error message.

In these cases it helps to know the type of failure, as the way to debug it
is different.

-EFAULT: It failed to read it the location. This means that the memory is
likely not even mapped in, or the pointer is way off.

-EINVAL: Means that what was read did not match what was expected (the code
was already updated, pointing to the wrong location, or simply the
calculation of what to expect is incorrect).

-EPERM: Means the write failed. What was read was expected, but the
permissions to write have not been updated properly.

Differentiating the three is crucial to looking at where the issue lies
when an ftrace_bug() triggers.

-- Steve