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) {
> 
> 




Reply via email to