[PATCH igt 7/9] msm/mapping: Handle larger gmem file size

[Rob Clark]

On Wed, Jul 2, 2025 at 12:48 AM Kamil Konieczny
<kamil.konieczny at linux.intel.com> wrote:
>
> Hi Rob,
> On 2025-07-01 at 11:01:06 -0700, Rob Clark wrote:
> > On Tue, Jul 1, 2025 at 9:21 AM Kamil Konieczny
> > <kamil.konieczny at linux.intel.com> wrote:
> > >
> > > Hi Rob,
> > > On 2025-06-30 at 11:09:01 -0700, Rob Clark wrote:
> > > > From: Rob Clark <rob.clark at oss.qualcomm.com>
> > > >
> > > > If running on a system with full desktop environment, the file will be
> > > > much larger than 0x4000 bytes.
> > > >
> > > > Signed-off-by: Rob Clark <rob.clark at oss.qualcomm.com>
> > > > ---
> > > >  tests/msm/msm_mapping.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/tests/msm/msm_mapping.c b/tests/msm/msm_mapping.c
> > > > index 03a9e814c931..978ea18375dd 100644
> > > > --- a/tests/msm/msm_mapping.c
> > > > +++ b/tests/msm/msm_mapping.c
> > > > @@ -122,7 +122,7 @@ endswith(const char *str, const char *end)
> > > >  static uint64_t
> > > >  get_bo_addr(int drm_fd, const char *name)
> > > >  {
> > > > -     char buf[0x4000];
> > > > +     char buf[0x80000];
> > >
> > > If it is that big why not malloc() it?
> >
> > in kernel, for sure.  But userspace stack size is multiple MB at least
> >
> > BR,
> > -R
>
> You are reading "gem" debugfs below, so this buffer in igt is in
> userspace, no need for it to be on stack. Btw you just discovered
> a bug in igt, imho this function should give sizeof(buf) to caller
> here to igt_debugfs_read():
>
> igt_debugfs_read(drm_fd, "gem", buf, sizeof(buf));

Actually igt_debugfs_read() is a convenience wrapper macro for
__igt_debugfs_read() which does just this.  But makes
igt_debugfs_read() more convenient for stack allocated buffers.

BR,
-R

>
> Also imho it should be calloc() to avoid garbage later on in buf.
> Or if it should read all of debugfs node, make it like:
>
> int igt_debugfs_read_all(int drm_fd, char *, char **buf);
>
> and let it alloc *buf, and later realloc this *buf if needed.
>
> Regards,
> Kamil
>
> >
> > > Regards,
> > > Kamil
> > >
> > > >       char *p = buf;
> > > >
> > > >       igt_debugfs_read(drm_fd, "gem", buf);
> > > > --
> > > > 2.50.0
> > > >