Re: [v6, 1/2] cxl: Add mechanism for delivering AFU driver specific events

Hi Ian,

Ian Munsie [off-list ref] writes:

Excerpts from Vaibhav Jain's message of 2016-06-20 14:20:16 +0530:

quoted

quoted

+int cxl_unset_driver_ops(struct cxl_context *ctx)
+{
+    if (atomic_read(&ctx->afu_driver_events))
+        return -EBUSY;
+
+    ctx->afu_driver_ops = NULL;

Need a write memory barrier so that afu_driver_ops isnt possibly called
after this store.

What situation do you think this will help? I haven't looked closely at
the last few iterations of this patch set, but if you're in a situation
where you might be racing with some code doing e.g.

if (ctx->afu_driver_ops)
	ctx->afu_driver_ops->something();

You have a race with or without a memory barrier. Ideally you would just
have the caller guarantee that it will only call cxl_unset_driver_ops if
no further calls to afu_driver_ops is possible, otherwise you may need
locking here which would be far from ideal.

Yes, agree that wmb wont save against the race condition mentioned and
this is much better handled with locking. But imho having a wmb is still
better compared to having no locking for this shared variable.

What exactly is the use case for this API? I'd vote to drop it if we can
do without it.

Agree with this. Functionality of this API can be merged with
cxl_set_driver_ops when called with NULL arg for cxl_afu_driver_ops.

~ Vaibhav