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;
MIME-Version: 1.0
References: <20210211080716.80982-1-info@alexander-lochmann.de>
 <CACT4Y+YwRE=YNQYmQ=7RWde33830YOYr5pEAoYbrofY2JG43MA@mail.gmail.com>
 <01a9177f-bfd5-251a-758f-d3c68bafd0cf@alexander-lochmann.de>
 <CACT4Y+ZPX43ihuL0TCiCY-ZNa4RmfwuieLb1XUDJEa4tELsUsQ@mail.gmail.com> <8841773d-c7d2-73aa-6fa6-fe496952f2ba@alexander-lochmann.de>
In-Reply-To: <8841773d-c7d2-73aa-6fa6-fe496952f2ba@alexander-lochmann.de>
From:   Dmitry Vyukov <dvyukov@google.com>
Date:   Tue, 16 Mar 2021 07:36:09 +0100
Message-ID: <CACT4Y+bV1rCZrtbJtPaKeY=2q8MW8bLsB95rdAeRyUDR3fMsDQ@mail.gmail.com>
Subject: Re: [PATCH] KCOV: Introduced tracing unique covered PCs
To:     Alexander Lochmann <info@alexander-lochmann.de>
Cc:     Andrey Konovalov <andreyknvl@google.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Andrew Morton <akpm@linux-foundation.org>,
        Wei Yongjun <weiyongjun1@huawei.com>,
        Maciej Grochowski <maciej.grochowski@pm.me>,
        kasan-dev <kasan-dev@googlegroups.com>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        syzkaller <syzkaller@googlegroups.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Mar 15, 2021 at 10:43 PM Alexander Lochmann
<info@alexander-lochmann.de> wrote:
> On 15.03.21 09:02, Dmitry Vyukov wrote:
> >>>>  static notrace unsigned long canonicalize_ip(unsigned long ip)
> >>>> @@ -191,18 +192,26 @@ void notrace __sanitizer_cov_trace_pc(void)
> >>>>         struct task_struct *t;
> >>>>         unsigned long *area;
> >>>>         unsigned long ip = canonicalize_ip(_RET_IP_);
> >>>> -       unsigned long pos;
> >>>> +       unsigned long pos, idx;
> >>>>
> >>>>         t = current;
> >>>> -       if (!check_kcov_mode(KCOV_MODE_TRACE_PC, t))
> >>>> +       if (!check_kcov_mode(KCOV_MODE_TRACE_PC | KCOV_MODE_UNIQUE_PC, t))
> >>>>                 return;
> >>>>
> >>>>         area = t->kcov_area;
> >>>> -       /* The first 64-bit word is the number of subsequent PCs. */
> >>>> -       pos = READ_ONCE(area[0]) + 1;
> >>>> -       if (likely(pos < t->kcov_size)) {
> >>>> -               area[pos] = ip;
> >>>> -               WRITE_ONCE(area[0], pos);
> >>>> +       if (likely(t->kcov_mode == KCOV_MODE_TRACE_PC)) {
> >>>
> >>> Does this introduce an additional real of t->kcov_mode?
> >>> If yes, please reuse the value read in check_kcov_mode.
> >> Okay. How do I get that value from check_kcov_mode() to the caller?
> >> Shall I add an additional parameter to check_kcov_mode()?
> >
> > Yes, I would try to add an additional pointer parameter for mode. I
> > think after inlining the compiler should be able to regestrize it.
> >
> Should kcov->mode be written directly to that ptr?
> Otherwise, it must be written to the already present variable mode, and
> than copied to the ptr (if not NULL).

I would expect that after inlining it won't make difference in
generated code. Is so, both options are fine. Whatever leads to a
cleaner code.