[PATCH] drm/gud: avoid divide error in gud_stats_debugfs

[PATCH] drm/gud: avoid divide error in gud_stats_debugfs

[YuanZhe Liu]

gud_stats_debugfs() computes the compression ratio by dividing the
accumulated uncompressed length by stats_actual_length. That counter is
zero until the first framebuffer flush has completed, so reading the
stats debugfs file on a display which negotiates LZ4 compression raises
a divide error before any frame has been sent. The wrap-around reset in
gud_flush_rect() also clears the counter for a moment, which a
concurrent read can observe.

Copy the counter into a local variable and skip the ratio computation
when it is zero.

Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Cc: stable@vger.kernel.org
Co-developed-by: Bin Yu <byu@xidian.edu.cn>
Signed-off-by: Bin Yu <byu@xidian.edu.cn>
Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
---
 drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c
index 3a1b9e2a2eaa..85ddb302d2ba 100644
--- a/drivers/gpu/drm/gud/gud_drv.c
+++ b/drivers/gpu/drm/gud/gud_drv.c
@@ -329,10 +329,17 @@ static int gud_stats_debugfs(struct seq_file *m, void *data)
 	seq_puts(m, "\n");
 
 	if (gdrm->compression) {
+		u64 actual_length = gdrm->stats_actual_length;
 		u64 remainder;
-		u64 ratio = div64_u64_rem(gdrm->stats_length, gdrm->stats_actual_length,
-					  &remainder);
-		u64 ratio_frac = div64_u64(remainder * 10, gdrm->stats_actual_length);
+		u64 ratio, ratio_frac;
+
+		if (!actual_length) {
+			seq_puts(m, "Compression ratio: n/a\n");
+			return 0;
+		}
+
+		ratio = div64_u64_rem(gdrm->stats_length, actual_length, &remainder);
+		ratio_frac = div64_u64(remainder * 10, actual_length);
 
 		seq_printf(m, "Compression ratio: %llu.%llu\n", ratio, ratio_frac);
 	}
-- 
2.34.1

Re: [PATCH] drm/gud: avoid divide error in gud_stats_debugfs

[Ruben Wauters]

[-- Attachment #1: Type: text/plain, Size: 2783 bytes --]

On Thu, 2026-10-29 at 15:33 +0800, YuanZhe Liu wrote:
> gud_stats_debugfs() computes the compression ratio by dividing the
> accumulated uncompressed length by stats_actual_length. That counter
> is
> zero until the first framebuffer flush has completed, so reading the
> stats debugfs file on a display which negotiates LZ4 compression
> raises
> a divide error before any frame has been sent. The wrap-around reset
> in
> gud_flush_rect() also clears the counter for a moment, which a
> concurrent read can observe.
> 
> Copy the counter into a local variable and skip the ratio computation
> when it is zero.
> 
> Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
> Cc: stable@vger.kernel.org
> Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
> Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
Acked-by: Ruben Wauters <rubenru09@aol.com>

Sashiko highlighted a potential issue with reading stats_actual_length
twice when a flush_rect occurs between them. This *could* still trigger
a divide error, however, this implementation is still better than
current. The best thing to do would be to put a READ_ONCE when
stats_actual_length is read.

This patch is better than the current implementation, and I don't think
many compilers would optimise it to two memory reads? But it is
possible, and compiler optimisation is a fairly complicated topic, so
if you do wish you can add the READ_ONCE, but I'm also happy for it to
go in without, as it is better than current.

Ruben
> ---
>  drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gud/gud_drv.c
> b/drivers/gpu/drm/gud/gud_drv.c
> index 3a1b9e2a2eaa..85ddb302d2ba 100644
> --- a/drivers/gpu/drm/gud/gud_drv.c
> +++ b/drivers/gpu/drm/gud/gud_drv.c
> @@ -329,10 +329,17 @@ static int gud_stats_debugfs(struct seq_file
> *m, void *data)
>  	seq_puts(m, "\n");
>  
>  	if (gdrm->compression) {
> +		u64 actual_length = gdrm->stats_actual_length;
>  		u64 remainder;
> -		u64 ratio = div64_u64_rem(gdrm->stats_length, gdrm-
> >stats_actual_length,
> -					  &remainder);
> -		u64 ratio_frac = div64_u64(remainder * 10, gdrm-
> >stats_actual_length);
> +		u64 ratio, ratio_frac;
> +
> +		if (!actual_length) {
> +			seq_puts(m, "Compression ratio: n/a\n");
> +			return 0;
> +		}
> +
> +		ratio = div64_u64_rem(gdrm->stats_length,
> actual_length, &remainder);
> +		ratio_frac = div64_u64(remainder * 10,
> actual_length);
>  
>  		seq_printf(m, "Compression ratio: %llu.%llu\n",
> ratio, ratio_frac);
>  	}

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 1028 bytes --]

Re: [PATCH] drm/gud: avoid divide error in gud_stats_debugfs

[Ruben Wauters]

[-- Attachment #1: Type: text/plain, Size: 3076 bytes --]

On Fri, 2026-10-30 at 22:07 +0000, Ruben Wauters wrote:
> On Thu, 2026-10-29 at 15:33 +0800, YuanZhe Liu wrote:
> > gud_stats_debugfs() computes the compression ratio by dividing the
> > accumulated uncompressed length by stats_actual_length. That
> > counter
> > is
> > zero until the first framebuffer flush has completed, so reading
> > the
> > stats debugfs file on a display which negotiates LZ4 compression
> > raises
> > a divide error before any frame has been sent. The wrap-around
> > reset
> > in
> > gud_flush_rect() also clears the counter for a moment, which a
> > concurrent read can observe.
> > 
> > Copy the counter into a local variable and skip the ratio
> > computation
> > when it is zero.
> > 
> > Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Bin Yu <byu@xidian.edu.cn>
> > Signed-off-by: Bin Yu <byu@xidian.edu.cn>
> > Co-developed-by: Cheng Wen <wencheng@xidian.edu.cn>
> > Signed-off-by: Cheng Wen <wencheng@xidian.edu.cn>
> > Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn>
> Acked-by: Ruben Wauters <rubenru09@aol.com>

Applied to drm-misc-fixes, thanks for the patch
> 
> Sashiko highlighted a potential issue with reading
> stats_actual_length
> twice when a flush_rect occurs between them. This *could* still
> trigger
> a divide error, however, this implementation is still better than
> current. The best thing to do would be to put a READ_ONCE when
> stats_actual_length is read.
> 
> This patch is better than the current implementation, and I don't
> think
> many compilers would optimise it to two memory reads? But it is
> possible, and compiler optimisation is a fairly complicated topic, so
> if you do wish you can add the READ_ONCE, but I'm also happy for it
> to
> go in without, as it is better than current.
> 
> Ruben
> > ---
> >  drivers/gpu/drm/gud/gud_drv.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/gud/gud_drv.c
> > b/drivers/gpu/drm/gud/gud_drv.c
> > index 3a1b9e2a2eaa..85ddb302d2ba 100644
> > --- a/drivers/gpu/drm/gud/gud_drv.c
> > +++ b/drivers/gpu/drm/gud/gud_drv.c
> > @@ -329,10 +329,17 @@ static int gud_stats_debugfs(struct seq_file
> > *m, void *data)
> >  	seq_puts(m, "\n");
> >  
> >  	if (gdrm->compression) {
> > +		u64 actual_length = gdrm->stats_actual_length;
> >  		u64 remainder;
> > -		u64 ratio = div64_u64_rem(gdrm->stats_length,
> > gdrm-
> > > stats_actual_length,
> > -					  &remainder);
> > -		u64 ratio_frac = div64_u64(remainder * 10, gdrm-
> > > stats_actual_length);
> > +		u64 ratio, ratio_frac;
> > +
> > +		if (!actual_length) {
> > +			seq_puts(m, "Compression ratio: n/a\n");
> > +			return 0;
> > +		}
> > +
> > +		ratio = div64_u64_rem(gdrm->stats_length,
> > actual_length, &remainder);
> > +		ratio_frac = div64_u64(remainder * 10,
> > actual_length);
> >  
> >  		seq_printf(m, "Compression ratio: %llu.%llu\n",
> > ratio, ratio_frac);
> >  	}

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 1028 bytes --]