On Thursday, 11 December 2025 12:24:31 CET Krzysztof Karas wrote:
> Hi Krzysztof,
>
> [...]
>
> > @@ -3375,7 +3360,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
> >
> > eb.exec = exec;
> > eb.vma = (struct eb_vma *)(exec + args->buffer_count + 1);
> > - eb.vma[0].vma = NULL;
> > +
> > + memset(eb.vma, 0x00, args->buffer_count * sizeof(struct eb_vma));
> > +
> > eb.batch_pool = NULL;
> >
> > eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
> > @@ -3584,7 +3571,16 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev,
> > void *data,
> > if (err)
> > return err;
> >
> > - /* Allocate extra slots for use by the command parser */
> > + /*
> > + * Allocate extra slots for use by the command parser.
> > + *
> > + * Note that this allocation handles two different arrays (the
> > + * exec2_list array, and the eventual eb.vma array introduced in
> > + * i915_gem_do_execubuffer()), that reside in virtually contiguous
> > + * memory. Also note that the allocation doesn't fill the area with
> > + * zeros (the first part doesn't need to be), but the second part only
> > + * is explicitly zeroed later in i915_gem_do_execbuffer().
> I get the gist of this comment, but I think you could reword the
> last sentence:
> "Also note that the allocation doesn't fill the area with zeros,
> because it is unnecessary for exec2_list array, and eb.vma is
> explicitly zeroed later in i915_gem_do_execbuffer()."
My preferred wording would look something like this:
Also note that the allocation intentionally doesn't fill the area with
zeros since the exec2_list array part is then fully overwritten with a
copy of user data before use. However, the eb.vma array part is still
expected to be initialized as needed by its user.
Thanks,
Janusz
>
> > + */
> > exec2_list = kvmalloc_array(count + 2, eb_element_size(),
> > __GFP_NOWARN | GFP_KERNEL);
> > if (exec2_list == NULL) {
>
>