Hi Ian,
Ian Munsie <imun...@au1.ibm.com> writes:
> Excerpts from Vaibhav Jain's message of 2016-06-20 14:20:16 +0530:
>> > +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
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
