2022-09-09 15:26:23

by Saurabh Singh Sengar

[permalink] [raw]
Subject: [PATCH] drm/hyperv: Add ratelimit on error message

Due to a full ring buffer, the driver may be unable to send updates to
the Hyper-V host. But outputing the error message can make the problem
worse because console output is also typically written to the frame
buffer.
Rate limiting the error message, also output the error code for additional
diagnosability.

Signed-off-by: Saurabh Sengar <[email protected]>
---
drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
index 76a182a..013a782 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
@@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg
VM_PKT_DATA_INBAND, 0);

if (ret)
- drm_err(&hv->dev, "Unable to send packet via vmbus\n");
+ drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret);

return ret;
}
--
1.8.3.1


2022-09-10 15:21:10

by Michael Kelley (LINUX)

[permalink] [raw]
Subject: RE: [PATCH] drm/hyperv: Add ratelimit on error message

From: Saurabh Sengar <[email protected]> Sent: Friday, September 9, 2022 8:10 AM
>
> Due to a full ring buffer, the driver may be unable to send updates to
> the Hyper-V host. But outputing the error message can make the problem
> worse because console output is also typically written to the frame
> buffer.
> Rate limiting the error message, also output the error code for additional
> diagnosability.
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> index 76a182a..013a782 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> @@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev,
> struct synthvid_msg
> VM_PKT_DATA_INBAND, 0);
>
> if (ret)
> - drm_err(&hv->dev, "Unable to send packet via vmbus\n");
> + drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret);
>
> return ret;
> }
> --
> 1.8.3.1

Reviewed-by: Michael Kelley <[email protected]>

2022-09-10 18:59:13

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH] drm/hyperv: Add ratelimit on error message

Hi

Am 09.09.22 um 17:09 schrieb Saurabh Sengar:
> Due to a full ring buffer, the driver may be unable to send updates to
> the Hyper-V host. But outputing the error message can make the problem
> worse because console output is also typically written to the frame
> buffer.
> Rate limiting the error message, also output the error code for additional
> diagnosability.
>
> Signed-off-by: Saurabh Sengar <[email protected]>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> index 76a182a..013a782 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> @@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg
> VM_PKT_DATA_INBAND, 0);
>
> if (ret)
> - drm_err(&hv->dev, "Unable to send packet via vmbus\n");
> + drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret);

I better option would probably be drm_err_once(). Then you'd get the
first error message and skip all others.

Best regards
Thomas

>
> return ret;
> }

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-09-11 16:15:39

by Saurabh Singh Sengar

[permalink] [raw]
Subject: Re: [PATCH] drm/hyperv: Add ratelimit on error message

On Sat, Sep 10, 2022 at 08:06:05PM +0200, Thomas Zimmermann wrote:
> Hi
>
> Am 09.09.22 um 17:09 schrieb Saurabh Sengar:
> >Due to a full ring buffer, the driver may be unable to send updates to
> >the Hyper-V host. But outputing the error message can make the problem
> >worse because console output is also typically written to the frame
> >buffer.
> >Rate limiting the error message, also output the error code for additional
> >diagnosability.
> >
> >Signed-off-by: Saurabh Sengar <[email protected]>
> >---
> > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> >index 76a182a..013a782 100644
> >--- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> >+++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> >@@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg
> > VM_PKT_DATA_INBAND, 0);
> > if (ret)
> >- drm_err(&hv->dev, "Unable to send packet via vmbus\n");
> >+ drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret);
>
> I better option would probably be drm_err_once(). Then you'd get the
> first error message and skip all others.

Thanks for your comment however the intention here is to limit the printk messages and break the chain
rather then printing only once. There can be cases where at different point of time we again get a
ring buffer full condition and we don't want to miss that. We should get the message for each of these
errror which are widely-separated in time not just the first time.

>
> Best regards
> Thomas
>
> > return ret;
> > }
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 N?rnberg, Germany
> (HRB 36809, AG N?rnberg)
> Gesch?ftsf?hrer: Ivo Totev



2022-09-29 17:18:07

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH] drm/hyperv: Add ratelimit on error message

On Sun, Sep 11, 2022 at 09:11:42AM -0700, Saurabh Singh Sengar wrote:
> On Sat, Sep 10, 2022 at 08:06:05PM +0200, Thomas Zimmermann wrote:
> > Hi
> >
> > Am 09.09.22 um 17:09 schrieb Saurabh Sengar:
> > >Due to a full ring buffer, the driver may be unable to send updates to
> > >the Hyper-V host. But outputing the error message can make the problem
> > >worse because console output is also typically written to the frame
> > >buffer.
> > >Rate limiting the error message, also output the error code for additional
> > >diagnosability.
> > >
> > >Signed-off-by: Saurabh Sengar <[email protected]>
> > >---
> > > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > >index 76a182a..013a782 100644
> > >--- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > >+++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> > >@@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg
> > > VM_PKT_DATA_INBAND, 0);
> > > if (ret)
> > >- drm_err(&hv->dev, "Unable to send packet via vmbus\n");
> > >+ drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret);
> >
> > I better option would probably be drm_err_once(). Then you'd get the
> > first error message and skip all others.
>
> Thanks for your comment however the intention here is to limit the printk messages and break the chain
> rather then printing only once. There can be cases where at different point of time we again get a
> ring buffer full condition and we don't want to miss that. We should get the message for each of these
> errror which are widely-separated in time not just the first time.

Applied to hyperv-next. Thanks.