Date: Sat, 29 Dec 2012 01:16:49 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: Re: svn commit: r244793 - in user/attilio/membarclean/dev: drm drm2 netmap virtio Message-ID: <20121228231649.GZ82219@kib.kiev.ua> In-Reply-To: <CAJ-FndB5aLcO6k3JOi7SuyvNLzvttD-pEE0MQXgMiP0hJKTyRA@mail.gmail.com> References: <201212282218.qBSMIfX2015054@svn.freebsd.org> <20121228223259.GX82219@kib.kiev.ua> <CAJ-FndB5aLcO6k3JOi7SuyvNLzvttD-pEE0MQXgMiP0hJKTyRA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--hW3Q5Oh0pcvvyJMS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Dec 28, 2012 at 10:55:47PM +0000, Attilio Rao wrote: > On Fri, Dec 28, 2012 at 10:32 PM, Konstantin Belousov > <kostikbel@gmail.com> wrote: > > On Fri, Dec 28, 2012 at 10:18:41PM +0000, Attilio Rao 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 operat= ion > >> 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 ne= ed > >> 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 > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- 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 > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- 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 > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- 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) =3D (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 > >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >> --- 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 =3D atomic_read(&dev->_vblank_count[crtc]); > >> *vblanktime =3D vblanktimestamp(dev, crtc, cur_vblank); > >> - rmb(); > >> - } while (cur_vblank !=3D atomic_read(&dev->_vblank_count[crtc])); > >> + } while (cur_vblank !=3D atomic_read_acq(&dev->_vblank_count[crt= c])); > >> > >> return cur_vblank; > >> } > > > > The drm/drm2 is the contributed code, maintained outside the tree, and I > > explicitely decided to keep the original approach. I will appreciate if > > this would be left as it is. >=20 > Aren't the stubs in atomic FreeBSD specific? > So it does rely on some sort of Linux layer emulation? > Right now my target is to get rid of rmb() and wmb() and garbage > collect them. If drm2 is Linux specific and needs to stay that way I > will make a different patch. *mb() are the Linux KPI, and almost any non-trivial ported driver depends on them. The mess which we already have in tree with the duplicated and sometimes wrong definitions of the barries is because we get this compat functions too late, and the driver authors added them locally. The functions should stay there, and their usage in the ported drivers too. Otherwise, the mess will reappear and continue to spread. I do prefer the drm code to stay similar to the Linux code. --hW3Q5Oh0pcvvyJMS Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQIcBAEBAgAGBQJQ3ihgAAoJEJDCuSvBvK1Bvm0QAIaO5lOauODCOv6x660i1K39 XHN3IdAWvrS98DarmiteqUumCQBeoyG2dbIPV3PZP5GOJCgQYlnaXQUcScebt9xF DXzVwV7XYDEXIhvgXg0aoX1QBsVSSrPDZYOMA4L0mEX7BAcGuUTzqIKCKX3Hz0VH M9FBCGY6oQ4lqyxJhAZVSu1Sw7eNGamLNWCypvAeI32c5UloAtzBKdX6/ad+5mvT J+79aAM+uUjJLJiZyorK9wPRn7uve2ldOGu9clSjnbTVxj21L1JEVyxYuMQWGw0K v55VTMJLLke+E6UuBceHE9/eYFXTR563N2TTPIuYN1jNNRn1vssLjmuvnuOsxyQp E1RipKlmG7hUymOPRCmQGpPFVrRtMMzpkCpNN3w9yEwoAJ1VFKrIJUJItCcct5PB CZD1FNKVy1JyH7Lj74cs8eiPMS7LlpaPigu/hfJ2hJL+MVEk/WN8Ug0vH0Z4lQRy BeJ7g7yl6Su4wMJNb6ri+QQ283Ijig0+XvGvqNgBEVYqdPmgrahIWTSqV052Xljy bVswBy8p+6biJ8n2aH7R9MdVXufEFXxHXflAJri0TkI/oyOn5wPCpvVET/1PIzBA t+a+LoW8s4PKUMW18u36+S/tGXLYLlcv+gTitxJjtMZlN8dR1mFO6kIfEeKXALRJ JFOHwb8Fl6dSiLWDlpOM =wiVS -----END PGP SIGNATURE----- --hW3Q5Oh0pcvvyJMS--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121228231649.GZ82219>