linux-kernel - Re: [PATCH] rpmsg: virtio_rpmsg_bus: use dev_warn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 29 Sep 2021 12:40:59 +0300
From:   Alexandru Ardelean <ardeleanalex@...il.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     linux-remoteproc@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Ohad Ben Cohen <ohad@...ery.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>
Subject: Re: [PATCH] rpmsg: virtio_rpmsg_bus: use dev_warn_ratelimited for msg
 with no recipient

On Wed, Sep 29, 2021 at 12:33 AM Bjorn Andersson
<bjorn.andersson@...aro.org> wrote:
>
> On Tue 28 Sep 08:29 CDT 2021, Alexandru Ardelean wrote:
>
> > From: Alexandru Ardelean <ardeleanalex@...il.com>
> >
> > Even though it may be user-space's fault for this error (some application
> > terminated or crashed without cleaning up it's endpoint), the rpmsg
> > communication should not overflow the syslog with too many messages.
> >
> > A dev_warn_ratelimited() seems like a good alternative in case this can
> > occur.
> >
>
> Is there anything a user could/should do when they see this entry in
> their log?

Not really, no.
The userspace application would need to respawn, or some systemd (or
similar process manager) would need to respawn the application it
should recover the state, and communication should resume normally.
I think this message is good mostly as informative.

>
> It doesn't look very actionable to me, should we perhaps degrade it
> further to just a dev_dbg()?

It's not actionable unfortunately.
But I feel it is useful to have this message, until the application recovers.
Mostly to be informative.
A more robust mechanism would be to setup some counters, where we
count the number of missed messages.
And then access this counter via sysfs or something.

The problem is that a high-rate of dev_warn() (during failure),
temporarily increases system CPU usage & load-average, making the
recovery a bit slower, because systemd-journald is processing these
messages from the kernel.
So, dev_dbg() would definitely help, but would also require us to bump
the system log-level to see the messages.
And if they occur and we don't see them, it causes more questions and
debugging, because people won't know for sure what the issue is.

Ultimately, dev_dbg() or dev_warn_rate_limited() are both fine.
The goal is to avoid the temporary increase in CPU load.
I just wanted to state my arguments for dev_warn_ratelimite() :)

Thank you
Alex

>
> Regards,
> Bjorn
>
> > Signed-off-by: Alexandru Ardelean <ardeleanalex@...il.com>
> > ---
> >  drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> > index 8e49a3bacfc7..546f0fb66f1d 100644
> > --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> > +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> > @@ -749,7 +749,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
> >               /* farewell, ept, we don't need you anymore */
> >               kref_put(&ept->refcount, __ept_release);
> >       } else
> > -             dev_warn(dev, "msg received with no recipient\n");
> > +             dev_warn_ratelimited(dev, "msg received with no recipient\n");
> >
> >       /* publish the real size of the buffer */
> >       rpmsg_sg_init(&sg, msg, vrp->buf_size);
> > --
> > 2.31.1
> >