svn commit: r244793 - in user/attilio/membarclean/dev: drm drm2 netmap virtio

[Bryan Venteicher]

On Fri, Dec 28, 2012 at 4:18 PM, Attilio Rao <attilio at freebsd.org> wrote:
> Author: attilio
> Date: Fri Dec 28 22:18:41 2012
> New Revision: 244793
> URL: http://svnweb.freebsd.org/changeset/base/244793
>
> Log:
>   - Remove rmb() usage from netmap and replace it with intended operation
>     to do actual memory fetching reads.
>   - GC unused DRM_WRITEMEMORYBARRIER() from drm and drm2.
>   - Use atomic_load_acq_*() in virtio and drm2 in places that don't need
>     to use rmb().
>
>   All these changes remove completely rmb() from MI code, with the
>   exception of cxgbe which will be hammered in a followup commit.
>
> Modified:
>   user/attilio/membarclean/dev/drm/drmP.h
>   user/attilio/membarclean/dev/drm2/drmP.h
>   user/attilio/membarclean/dev/drm2/drm_atomic.h
>   user/attilio/membarclean/dev/drm2/drm_irq.c
>   user/attilio/membarclean/dev/netmap/netmap.c
>   user/attilio/membarclean/dev/virtio/virtqueue.c
>
> Modified: user/attilio/membarclean/dev/drm/drmP.h
> ==============================================================================
> --- user/attilio/membarclean/dev/drm/drmP.h     Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/drm/drmP.h     Fri Dec 28 22:18:41 2012        (r244793)
> @@ -241,11 +241,9 @@ typedef u_int32_t u32;
>  typedef u_int16_t u16;
>  typedef u_int8_t u8;
>
> -/* DRM_READMEMORYBARRIER() prevents reordering of reads.
> - * DRM_WRITEMEMORYBARRIER() prevents reordering of writes.
> +/* DRM_WRITEMEMORYBARRIER() prevents reordering of writes.
>   * DRM_MEMORYBARRIER() prevents reordering of reads and writes.
>   */
> -#define DRM_READMEMORYBARRIER()                rmb()
>  #define DRM_WRITEMEMORYBARRIER()       wmb()
>  #define DRM_MEMORYBARRIER()            mb()
>
>
> Modified: user/attilio/membarclean/dev/drm2/drmP.h
> ==============================================================================
> --- user/attilio/membarclean/dev/drm2/drmP.h    Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/drm2/drmP.h    Fri Dec 28 22:18:41 2012        (r244793)
> @@ -263,11 +263,9 @@ typedef int32_t s32;
>  typedef int16_t s16;
>  typedef int8_t s8;
>
> -/* DRM_READMEMORYBARRIER() prevents reordering of reads.
> - * DRM_WRITEMEMORYBARRIER() prevents reordering of writes.
> +/* DRM_WRITEMEMORYBARRIER() prevents reordering of writes.
>   * DRM_MEMORYBARRIER() prevents reordering of reads and writes.
>   */
> -#define DRM_READMEMORYBARRIER()                rmb()
>  #define DRM_WRITEMEMORYBARRIER()       wmb()
>  #define DRM_MEMORYBARRIER()            mb()
>
>
> Modified: user/attilio/membarclean/dev/drm2/drm_atomic.h
> ==============================================================================
> --- user/attilio/membarclean/dev/drm2/drm_atomic.h      Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/drm2/drm_atomic.h      Fri Dec 28 22:18:41 2012        (r244793)
> @@ -38,6 +38,7 @@ typedef u_int32_t atomic_t;
>
>  #define atomic_set(p, v)       (*(p) = (v))
>  #define atomic_read(p)         (*(p))
> +#define atomic_read_acq(p)     atomic_load_acq_int(p)
>  #define atomic_inc(p)          atomic_add_int(p, 1)
>  #define atomic_dec(p)          atomic_subtract_int(p, 1)
>  #define atomic_add(n, p)       atomic_add_int(p, n)
>
> Modified: user/attilio/membarclean/dev/drm2/drm_irq.c
> ==============================================================================
> --- user/attilio/membarclean/dev/drm2/drm_irq.c Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/drm2/drm_irq.c Fri Dec 28 22:18:41 2012        (r244793)
> @@ -701,8 +701,7 @@ u32 drm_vblank_count_and_time(struct drm
>         do {
>                 cur_vblank = atomic_read(&dev->_vblank_count[crtc]);
>                 *vblanktime = vblanktimestamp(dev, crtc, cur_vblank);
> -               rmb();
> -       } while (cur_vblank != atomic_read(&dev->_vblank_count[crtc]));
> +       } while (cur_vblank != atomic_read_acq(&dev->_vblank_count[crtc]));
>
>         return cur_vblank;
>  }
>
> Modified: user/attilio/membarclean/dev/netmap/netmap.c
> ==============================================================================
> --- user/attilio/membarclean/dev/netmap/netmap.c        Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/netmap/netmap.c        Fri Dec 28 22:18:41 2012        (r244793)
> @@ -98,6 +98,9 @@ MALLOC_DEFINE(M_NETMAP, "netmap", "Netwo
>  #include <net/netmap.h>
>  #include <dev/netmap/netmap_kern.h>
>
> +#define        MEMFETCH(var)                                                   \
> +       (__DEVOLATILE(__typeof(var), *((volatile __typeof(var) *)&var)))
> +
>  u_int netmap_total_buffers;
>  u_int netmap_buf_size;
>  char *netmap_buffer_base;      /* address of an invalid buffer */
> @@ -1081,10 +1084,7 @@ error:
>                         error = ENXIO;
>                         break;
>                 }
> -               rmb(); /* make sure following reads are not from cache */
> -
> -
> -               ifp = priv->np_ifp;     /* we have a reference */
> +               ifp = MEMFETCH(priv->np_ifp);
>
>                 if (ifp == NULL) {
>                         D("Internal error: nifp != NULL && ifp == NULL");
> @@ -1194,9 +1194,7 @@ netmap_poll(struct cdev *dev, int events
>                 D("No if registered");
>                 return POLLERR;
>         }
> -       rmb(); /* make sure following reads are not from cache */
> -
> -       ifp = priv->np_ifp;
> +       ifp = MEMFETCH(priv->np_ifp);
>         // XXX check for deleting() ?
>         if ( (ifp->if_capenable & IFCAP_NETMAP) == 0)
>                 return POLLERR;
>
> Modified: user/attilio/membarclean/dev/virtio/virtqueue.c
> ==============================================================================
> --- user/attilio/membarclean/dev/virtio/virtqueue.c     Fri Dec 28 22:06:50 2012        (r244792)
> +++ user/attilio/membarclean/dev/virtio/virtqueue.c     Fri Dec 28 22:18:41 2012        (r244793)
> @@ -525,8 +525,7 @@ virtqueue_dequeue(struct virtqueue *vq,
>         used_idx = vq->vq_used_cons_idx++ & (vq->vq_nentries - 1);
>         uep = &vq->vq_ring.used->ring[used_idx];
>
> -       rmb();
> -       desc_idx = (uint16_t) uep->id;
> +       desc_idx = (uint16_t)atomic_load_acq_32(&uep->id);
>         if (len != NULL)
>                 *len = uep->len;
>

VirtIO uses 16-bit fields which makes it hard to adapt to MI
atomic(9). The field here happens to be 32-bits, but that is for
alignment purposes. I have a patch from a couple months back that
switches VirtIO to atomic_*(), but the mb()'s are still #ifdef in
there for the eventual support of arch's without 16-bit atomic(9) (and
the patch needs a bit of followup work).