Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220331070115.29421-1-bjorn.ardo@axis.com> <CABb+yY1zrHYLBjY_EoC7bkTsWcKOhzCjp-5vgvqP0HEAJicdXQ@mail.gmail.com>
 <c4aa28b2-5550-ed39-3869-556b451ff54b@axis.com> <CABb+yY1BNsdMq7CNOBDk3sn7uvpL4=-fT7eOcbuL-+Yjz+iqHw@mail.gmail.com>
 <487bbd00-d763-0a86-9984-1dfd957fbb38@axis.com> <e3abb8c0-a42c-4eea-993e-3c8fcce0ae64@axis.com>
In-Reply-To: <e3abb8c0-a42c-4eea-993e-3c8fcce0ae64@axis.com>
From:   Jassi Brar <jassisinghbrar@gmail.com>
Date:   Mon, 23 May 2022 14:35:41 -0500
Message-ID: <CABb+yY2EXGFK-ayV=-VRmEv5YFQWO5X+Xu2=mgcVegkx8MDogg@mail.gmail.com>
Subject: Re: [PATCH] mailbox: forward the hrtimer if not queued and under a lock
To:     Bjorn Ardo <bjorn.ardo@axis.com>
Cc:     kernel <kernel@axis.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, May 23, 2022 at 6:56 AM Bjorn Ardo <bjorn.ardo@axis.com> wrote:
>
> Hi again,
>
>
> On 4/20/22 10:28, Bjorn Ardo wrote:
> >
> >
> > Our current solution are using 4 different mailbox channels
> > asynchronously. The code is part of a larger system, but I can put
> > down some time and try and extract the relevant parts if you still
> > think this is a client issue? But with my current understanding of the
> > code, the race between msg_submit() and txdone_hrtimer() is quite
> > clear, and with my proposed patch that removes this race we have be
> > able to run for very long time without any problems (that is several
> > days). Without the fix we get the race after 5-10 min.
> >
> >
> >
>
> I do not know if you have had any time to review my comments yet, but we
> have created some examples to trigger the error.
>
Thanks, but your last explanation was enough. The logic seems fine.
I was hoping someone impacted by the commit may chime in with a tested/acke=
d by.
I think I'll pick it up now.

Thanks.


>
> With the attached testmodule mailbox-loadtest.c I can trigger the error
> by attaching it to the two sides of an mailbox with the following
> devicetree code:
>
>          mboxtest1 {
>                  compatible =3D "mailbox-loadtest";
>                  mbox-names =3D "ping", "pong";
>                  mboxes =3D <&mbox_loop_pri 0 &mbox_loop_pri 1>;
>          };
>
>          mboxtest2 {
>                  compatible =3D "mailbox-loadtest";
>                  mbox-names =3D "pong", "ping";
>                  mboxes =3D <&mbox_loop_scd 0 &mbox_loop_scd 1>;
>          };
>
>
> After that I create load on the mailbox by running (or respectively
> system) the following:
>
> while echo 1 > /sys/kernel/debug/mboxtest1/ping ; do
> usleep 1
> done
>
> while echo 1 > /sys/kernel/debug/mboxtest2/ping ; do
> usleep 50000
> done
>
> After a few minutes (normally 2-5) I get errors.
>
>
> Using the patch I sent earlier the errors goes away.
>
>
> We also have created a mailbox-loopback.c that is a loopback mailbox
> that can be used on the same system (to make testing easier on systems
> that does not have a hardware mailbox), it is also attached. This can be
> probed by the following devicetree code:
>
>          mbox_loop_pri: mailbox_loop_pri {
>                  compatible =3D "mailbox-loopback";
>                  #mbox-cells =3D <1>;
>                  side =3D <0>;
>          };
>          mbox_loop_scd: mailbox_loop_scd {
>                  compatible =3D "mailbox-loopback";
>                  #mbox-cells =3D <1>;
>                  side =3D <1>;
>          };
>
> And with this loopback mailbox we have also been able to reproduce the
> errors without the patch applied.
>
>
> Best Regards,
>
> Bj=C3=B6rn
>