| Message ID | 20180122200003.6665-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Christian König (2018-01-22 20:00:03) > We need to set shared_count even if we already have a fence to wait for. > > v2: init i to -1 as well > > Signed-off-by: Christian König <christian.koenig@amd.com> > Cc: stable@vger.kernel.org > Tested-by: Lyude Paul <lyude@redhat.com> > Reviewed-by: Lyude Paul <lyude@redhat.com> > --- > drivers/dma-buf/reservation.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index 461afa9febd4..314eb1071cce 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -484,13 +484,15 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, > unsigned long timeout) > { > struct dma_fence *fence; > - unsigned seq, shared_count, i = 0; > + unsigned seq, shared_count; > long ret = timeout ? timeout : 1; > + int i; > > retry: > shared_count = 0; > seq = read_seqcount_begin(&obj->seq); > rcu_read_lock(); > + i = -1; Could be before the seqlock, but Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Are we at the point where just grabbing the snapshot of fences with your new get_fences_rcu() returning a single array will be simpler? (It also has the change in behaviour of not updating the snapshot across the long lived wait.) If you want to avoid the kmalloc, we could teach it populate the caller's stack allocated array first. -Chris
Am 22.01.2018 um 21:09 schrieb Chris Wilson: > Quoting Christian König (2018-01-22 20:00:03) >> We need to set shared_count even if we already have a fence to wait for. >> >> v2: init i to -1 as well >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Cc: stable@vger.kernel.org >> Tested-by: Lyude Paul <lyude@redhat.com> >> Reviewed-by: Lyude Paul <lyude@redhat.com> >> --- >> drivers/dma-buf/reservation.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c >> index 461afa9febd4..314eb1071cce 100644 >> --- a/drivers/dma-buf/reservation.c >> +++ b/drivers/dma-buf/reservation.c >> @@ -484,13 +484,15 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, >> unsigned long timeout) >> { >> struct dma_fence *fence; >> - unsigned seq, shared_count, i = 0; >> + unsigned seq, shared_count; >> long ret = timeout ? timeout : 1; >> + int i; >> >> retry: >> shared_count = 0; >> seq = read_seqcount_begin(&obj->seq); >> rcu_read_lock(); >> + i = -1; > Could be before the seqlock, but > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > > Are we at the point where just grabbing the snapshot of fences with your > new get_fences_rcu() returning a single array will be simpler? (It also > has the change in behaviour of not updating the snapshot across the long > lived wait.) Yeah, thought about that as well. > If you want to avoid the kmalloc, we could teach it populate the > caller's stack allocated array first. Something like a helper function which just grabs all fences and puts them in an array of size n? And if the array is to small we return -ENOSPC and the caller needs to resize it. Good idea, putting this on my TODO list, but no idea when I can get around to actually doing it. Christian. > -Chris
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 461afa9febd4..314eb1071cce 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -484,13 +484,15 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, unsigned long timeout) { struct dma_fence *fence; - unsigned seq, shared_count, i = 0; + unsigned seq, shared_count; long ret = timeout ? timeout : 1; + int i; retry: shared_count = 0; seq = read_seqcount_begin(&obj->seq); rcu_read_lock(); + i = -1; fence = rcu_dereference(obj->fence_excl); if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { @@ -506,14 +508,14 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, fence = NULL; } - if (!fence && wait_all) { + if (wait_all) { struct reservation_object_list *fobj = rcu_dereference(obj->fence); if (fobj) shared_count = fobj->shared_count; - for (i = 0; i < shared_count; ++i) { + for (i = 0; !fence && i < shared_count; ++i) { struct dma_fence *lfence = rcu_dereference(fobj->shared[i]); if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,