Hi Niranjana, On Mon, Oct 03, 2022 at 02:06:18PM -0700, Niranjana Vishwanathapura wrote: > On Mon, Oct 03, 2022 at 05:53:37PM +0200, Andi Shyti wrote: > > Hi Niranjana, > > > > [...] > > > > > + for_each_child(ce, child) { > > > + err = intel_context_pin_ww(child, ww); > > > + GEM_BUG_ON(err); /* perma-pinned should incr a counter */ > > > + } > > > + > > > + for_each_child(ce, child) { > > > + err = eb_pin_timeline(child, throttle, nonblock); > > > + if (err) > > > + goto unwind; > > > + ++i; > > > + } > > > > any reason for having two separate for_each_child here? > > > > This part is ported as is from i915_gem_execbuffer.c. > Probably the author found it easy to unwind in case of error. yes... yes... I know... but these hard copies are also a good occasion to do some refactoring on the original code... but anyway, let's keep this simple... I forgot earlier: Reviewed-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> Andi