Skip to content

Commit

Permalink
UPSTREAM: dma-buf: make fence sequence numbers 64 bit v2
Browse files Browse the repository at this point in the history
For a lot of use cases we need 64bit sequence numbers. Currently drivers
overload the dma_fence structure to store the additional bits.

Stop doing that and make the sequence number in the dma_fence always
64bit.

For compatibility with hardware which can do only 32bit sequences the
comparisons in __dma_fence_is_later only takes the lower 32bits as significant
when the upper 32bits are all zero.

v2: change the logic in __dma_fence_is_later

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
Link: https://patchwork.freedesktop.org/patch/266927/
(cherry picked from commit b312d8c)

Context: 0day reports:
    drivers/gpu/drm/virtio/virtgpu_fence.c:50:26: warning:
	format '%llu' expects argument of type 'long long unsigned int',
	but argument 4 has type 'unsigned int'

Analysis shows that commit 2f22ba2 ("UPSTREAM: drm/virtio: set seqno
for dma-fence") moved sequence numbers from struct virtio_gpu_fence to
struct dma_fence. In the upstream kernel, the sequence number in struct
dma_fence is 64 bit wide. In chromeos-4.19, it is 32 bit, effecively
reducing the sequence number space. We could "fix" the problem reported
by 0day by replacing "%llu" with "%u", but that seems wrong. Backporting
this commit, which introduces 64 bit sequence numbers in struct dma_fence,
seems to be more appropriate.

BUG=chromium:924405
TEST=glxgears in Crostini

Reported-by: kbuild test robot <lkp@intel.com>
Change-Id: Ib00eeefed33e97785b202249c03ab1155008afd5
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1704173
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Stéphane Marchesin <marcheu@chromium.org>
  • Loading branch information
ChristianKoenigAMD authored and Commit Bot committed Jul 16, 2019
1 parent 1472299 commit 88445be
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 16 deletions.
2 changes: 1 addition & 1 deletion drivers/dma-buf/dma-fence.c
Expand Up @@ -615,7 +615,7 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
*/
void
dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
spinlock_t *lock, u64 context, unsigned seqno)
spinlock_t *lock, u64 context, u64 seqno)
{
BUG_ON(!lock);
BUG_ON(!ops || !ops->get_driver_name || !ops->get_timeline_name);
Expand Down
2 changes: 1 addition & 1 deletion drivers/dma-buf/sw_sync.c
Expand Up @@ -172,7 +172,7 @@ static bool timeline_fence_enable_signaling(struct dma_fence *fence)
static void timeline_fence_value_str(struct dma_fence *fence,
char *str, int size)
{
snprintf(str, size, "%d", fence->seqno);
snprintf(str, size, "%lld", fence->seqno);
}

static void timeline_fence_timeline_value_str(struct dma_fence *fence,
Expand Down
4 changes: 2 additions & 2 deletions drivers/dma-buf/sync_file.c
Expand Up @@ -144,7 +144,7 @@ char *sync_file_get_name(struct sync_file *sync_file, char *buf, int len)
} else {
struct dma_fence *fence = sync_file->fence;

snprintf(buf, len, "%s-%s%llu-%d",
snprintf(buf, len, "%s-%s%llu-%lld",
fence->ops->get_driver_name(fence),
fence->ops->get_timeline_name(fence),
fence->context,
Expand Down Expand Up @@ -258,7 +258,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a,

i_b++;
} else {
if (pt_a->seqno - pt_b->seqno <= INT_MAX)
if (__dma_fence_is_later(pt_a->seqno, pt_b->seqno))
add_fence(fences, &i, pt_a);
else
add_fence(fences, &i, pt_b);
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c
Expand Up @@ -388,7 +388,7 @@ void amdgpu_sa_bo_dump_debug_info(struct amdgpu_sa_manager *sa_manager,
soffset, eoffset, eoffset - soffset);

if (i->fence)
seq_printf(m, " protected by 0x%08x on context %llu",
seq_printf(m, " protected by 0x%016llx on context %llu",
i->fence->seqno, i->fence->context);

seq_printf(m, "\n");
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/i915_sw_fence.c
Expand Up @@ -393,7 +393,7 @@ static void timer_i915_sw_fence_wake(struct timer_list *t)
if (!fence)
return;

pr_notice("Asynchronous wait on fence %s:%s:%x timed out (hint:%pS)\n",
pr_notice("Asynchronous wait on fence %s:%s:%llx timed out (hint:%pS)\n",
cb->dma->ops->get_driver_name(cb->dma),
cb->dma->ops->get_timeline_name(cb->dma),
cb->dma->seqno,
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/intel_engine_cs.c
Expand Up @@ -1238,7 +1238,7 @@ static void print_request(struct drm_printer *m,

x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf));

drm_printf(m, "%s%x%s [%llx:%x]%s @ %dms: %s\n",
drm_printf(m, "%s%x%s [%llx:%llx]%s @ %dms: %s\n",
prefix,
rq->global_seqno,
i915_request_completed(rq) ? "!" : "",
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/vgem/vgem_fence.c
Expand Up @@ -53,13 +53,13 @@ static void vgem_fence_release(struct dma_fence *base)

static void vgem_fence_value_str(struct dma_fence *fence, char *str, int size)
{
snprintf(str, size, "%u", fence->seqno);
snprintf(str, size, "%llu", fence->seqno);
}

static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
int size)
{
snprintf(str, size, "%u",
snprintf(str, size, "%llu",
dma_fence_is_signaled(fence) ? fence->seqno : 0);
}

Expand Down
22 changes: 15 additions & 7 deletions include/linux/dma-fence.h
Expand Up @@ -77,7 +77,7 @@ struct dma_fence {
struct list_head cb_list;
spinlock_t *lock;
u64 context;
unsigned seqno;
u64 seqno;
unsigned long flags;
ktime_t timestamp;
int error;
Expand Down Expand Up @@ -244,7 +244,7 @@ struct dma_fence_ops {
};

void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
spinlock_t *lock, u64 context, unsigned seqno);
spinlock_t *lock, u64 context, u64 seqno);

void dma_fence_release(struct kref *kref);
void dma_fence_free(struct dma_fence *fence);
Expand Down Expand Up @@ -414,9 +414,17 @@ dma_fence_is_signaled(struct dma_fence *fence)
* Returns true if f1 is chronologically later than f2. Both fences must be
* from the same context, since a seqno is not common across contexts.
*/
static inline bool __dma_fence_is_later(u32 f1, u32 f2)
static inline bool __dma_fence_is_later(u64 f1, u64 f2)
{
return (int)(f1 - f2) > 0;
/* This is for backward compatibility with drivers which can only handle
* 32bit sequence numbers. Use a 64bit compare when any of the higher
* bits are none zero, otherwise use a 32bit compare with wrap around
* handling.
*/
if (upper_32_bits(f1) || upper_32_bits(f2))
return f1 > f2;

return (int)(lower_32_bits(f1) - lower_32_bits(f2)) > 0;
}

/**
Expand Down Expand Up @@ -547,21 +555,21 @@ u64 dma_fence_context_alloc(unsigned num);
do { \
struct dma_fence *__ff = (f); \
if (IS_ENABLED(CONFIG_DMA_FENCE_TRACE)) \
pr_info("f %llu#%u: " fmt, \
pr_info("f %llu#%llu: " fmt, \
__ff->context, __ff->seqno, ##args); \
} while (0)

#define DMA_FENCE_WARN(f, fmt, args...) \
do { \
struct dma_fence *__ff = (f); \
pr_warn("f %llu#%u: " fmt, __ff->context, __ff->seqno, \
pr_warn("f %llu#%llu: " fmt, __ff->context, __ff->seqno,\
##args); \
} while (0)

#define DMA_FENCE_ERR(f, fmt, args...) \
do { \
struct dma_fence *__ff = (f); \
pr_err("f %llu#%u: " fmt, __ff->context, __ff->seqno, \
pr_err("f %llu#%llu: " fmt, __ff->context, __ff->seqno, \
##args); \
} while (0)

Expand Down

0 comments on commit 88445be

Please sign in to comment.