Re: [dpdk-dev] [PATCH v3 2/3] net/virtio: update memory ordering comment for vq notify

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi Gavin,

On 1/10/19 9:19 AM, Gavin Hu (Arm Technology China) wrote:
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ilya Maximets
>> Sent: Wednesday, January 9, 2019 10:50 PM
>> To: dev@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>;
>> Michael S . Tsirkin <mst@redhat.com>; Xiao Wang
>> <xiao.w.wang@intel.com>
>> Cc: Tiwei Bie <tiwei.bie@intel.com>; Zhihong Wang
>> <zhihong.wang@intel.com>; jfreimann@redhat.com; Jason Wang
>> <jasowang@redhat.com>; xiaolong.ye@intel.com;
>> alejandro.lucero@netronome.com; Ilya Maximets
>> <i.maximets@samsung.com>
>> Subject: [dpdk-dev] [PATCH v3 2/3] net/virtio: update memory ordering
>> comment for vq notify
>>
>> We're not using io ports in case of modern device even on IA.
>> Also, this comment useless for other architectures.
> 
> Agree, it should be architecture neutral.
> 
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>   drivers/net/virtio/virtqueue.h | 9 ++++-----
>>   1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
>> index dffa03669..53aeac238 100644
>> --- a/drivers/net/virtio/virtqueue.h
>> +++ b/drivers/net/virtio/virtqueue.h
>> @@ -437,14 +437,13 @@ virtqueue_kick_prepare_packed(struct virtqueue
>> *vq)
>>   	return flags != RING_EVENT_FLAGS_DISABLE;
>>   }
>>
>> +/*
>> + * virtqueue_kick_prepare*() or the virtio_wmb() should be called
>> + * before this function to be sure that all the data is visible to vhost.
>> + */
> 
> C11 _atomic APIs are preferred for new code, other than wmb or rmb,  could you work on that?

Thanks for the review.
-rc2 deadline is today, so I may apply this series as is if no reply
from Ilya today. If a change is agreed, it could be made on top for
-rc3.

Regards,
Maxime
>>   static inline void
>>   virtqueue_notify(struct virtqueue *vq)
>>   {
>> -	/*
>> -	 * Ensure updated avail->idx is visible to host.
>> -	 * For virtio on IA, the notificaiton is through io port operation
>> -	 * which is a serialization instruction itself.
>> -	 */
>>   	VTPCI_OPS(vq->hw)->notify_queue(vq->hw, vq);
>>   }
>>
>> --
>> 2.17.1
>