Re: [PATCH v6 1/3] drm/i915: Update object placement flags to be mutable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 23, 2021 at 5:38 PM Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx> wrote:
>
> Thanks for reviewing, Daniel.
>
> On 6/23/21 5:09 PM, Daniel Vetter wrote:
> >
> >>
> >> +    unsigned int mem_flags:2;
> > Is the entire bitfield array all protected by dma_resv_lock? If not I'd
> > just go with a full field, avoids headaches and all that.
> >
> > Also kerneldoc for this would be really sweet. Means some work to get it
> > going,
>
> Yeah, late documentation review comments after v9 ought to be forbidden ;)

Well I think we should have locking and all that documented from the
start maybe :-P

But yeah I know it's a bit late, so totally fine if that's done as a
follow up on top. But for new stuff or revised stuff we need to start
somewhere, and "maybe later when we have time" just never cuts it ...

>
> > but somewhere we need to stop hacking together undocumented ad-hoc
> > locking schemes :-/
>
> Hmm, this was intended to replace the change of and access of object ops
> *without* the lock held and with proper asserts added in the accessors,
> so it was not really intended to be an ad-hoc locking scheme, It's
> simply placement related things are updated under the lock.

Yeah this was more meant as a general comment. E.g. in struct i915_vma
we now have the situation that we have 2 overlapping locking schemes,
and it's almost impossible to figure out which is infect for which
pieces. I'd like to avoid that if at all possible.

> I'll update the code and resend.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux