Re: bug fix for registers debugfs file implementation [RFC]

From: Mark Brown
Date: Fri Apr 21 2017 - 14:33:16 EST


On Sat, Apr 01, 2017 at 02:13:41AM -0700, noman pouigt wrote:
> Current registers debugfs file implementation doesn't
> handle if the size exceeds 4k. It just dumps 4k of registers.
> Fix this by using the seq_file which already handles
> the file offset logic instead of reinventing the wheel.
>
> I am wondering if there is any issue is doing below which
> I am not aware of?

If I remember correctly this is done the way it is because seq_file has
to iterate through the entire file to get to the point being read by the
application. This is a *very* big overhead for some applications (like
monitoring some registers to see what they're doing) on bigger devices,
especially if there's lots of uncached stuff and the reads have to go to
the hardware. With the current code you can open the file, seek to the
registers you're interested in and read only them. You'll notice that
the other debug files have been converted over to seq_file as they're
pure software and there's less reason to repeatedly read them.

I'm also very surprised that this is failing for you as I know this code
has been fairly heavily exercised with devices with very large register
maps much larger than 4k and my own testing is mainly doing cats which
seemed to work last time I tried and should be iterating over the 4k
boundary... what's the actual failure mode? I'm guessing it's
something if we end a register exactly on a page boundary or something?

Also your patch is completely tab/space mangled so it's not appliable.

> +static void regmap_debugfs_stop(struct seq_file *s, void *v)
> +{
> +}

The need for empty functions is worrying...

Attachment: signature.asc
Description: PGP signature