Date: Tue, 5 Mar 2013 17:37:36 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Jean-S?bastien P?dron <dumbbell@FreeBSD.org> Cc: freebsd-current@freebsd.org, "J.R. Oldroyd" <jr@opal.com> Subject: Re: r247835: drm2 code breaks buildkernel Message-ID: <20130305153736.GH3794@kib.kiev.ua> In-Reply-To: <5135FD78.1050608@FreeBSD.org> References: <5135C70B.50906@zedat.fu-berlin.de> <5135CD0E.8040801@dumbbell.fr> <5135DE36.9010303@zedat.fu-berlin.de> <20130305123016.GE1483@glenbarber.us> <5135FD78.1050608@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--jaoouwwPWoQSJZYp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Tue, Mar 05, 2013 at 03:13:12PM +0100, Jean-S?bastien P?dron wrote:
> On 05.03.2013 13:30, Glen Barber wrote:
> > dev/drm2/ttm/ttm_lock.h:208: warning: redundant redeclaration of 'ttm_w=
rite_unlock' [-Wredundant-decls]
> > dev/drm2/ttm/ttm_lock.h:134: warning: previous declaration of 'ttm_writ=
e_unlock' was here
> > dev/drm2/ttm/ttm_lock.h:220: warning: redundant redeclaration of 'ttm_w=
rite_lock' [-Wredundant-decls]
> > dev/drm2/ttm/ttm_lock.h:146: warning: previous declaration of 'ttm_writ=
e_lock' was here
>=20
> Those redundant declarations weren't spotted by clang.
>=20
> Konstantin, would you like me to commit the fix for this? And we need to
> upstream it too.
I am somewhat sceptical about upstreaming. I reported a real bug in the
ttm_lock with the trivial fix when I read the code for porting. Bug was
confirmed by Linux people, but no action was taken.
>=20
> > dev/drm2/ttm/ttm_page_alloc.c:122: warning: declaration does not declar=
e anything
> > dev/drm2/ttm/ttm_page_alloc.c:123: warning: declaration does not declar=
e anything
>=20
> These errors and the following are caused by unnamed structs and unions
> inside another struct:
>=20
> struct ttm_pool_manager {
> ...
>=20
> union {
> struct ttm_page_pool pools[NUM_POOLS];
> struct {
> ...
> } ;
> };
> };
>=20
> With default options, clang accepts this but apparently, not gcc.
>=20
> I would like an opinion from the toolchain gurus, because I don't know
> what's the proper way to fix this one.
>=20
> J.R. Oldroyd CC'd, because he started to work on radeonkms backport to 9
> and faced exactly those issues.
The patch below is supposed to fix double declaration (it is pathetic that
clang silently accepts this, while issuing countless useless warnings).
Also there is a usual workaround for the anonimous union/struct issue.
Glen neglected to even mention that he used gcc (is it true ?), as well
as to show the command line invocation of the compiler. Hopefully the
patch helps.
diff --git a/sys/dev/drm2/ttm/ttm_lock.h b/sys/dev/drm2/ttm/ttm_lock.h
index ac8159e..6d45457 100644
--- a/sys/dev/drm2/ttm/ttm_lock.h
+++ b/sys/dev/drm2/ttm/ttm_lock.h
@@ -125,27 +125,6 @@ extern int ttm_read_lock(struct ttm_lock *lock, bool i=
nterruptible);
extern int ttm_read_trylock(struct ttm_lock *lock, bool interruptible);
=20
/**
- * ttm_write_unlock
- *
- * @lock: Pointer to a struct ttm_lock
- *
- * Releases a write lock.
- */
-extern void ttm_write_unlock(struct ttm_lock *lock);
-
-/**
- * ttm_write_lock
- *
- * @lock: Pointer to a struct ttm_lock
- * @interruptible: Interruptible sleeping while waiting for a lock.
- *
- * Takes the lock in write mode.
- * Returns:
- * -ERESTARTSYS If interrupted by a signal and interruptible is true.
- */
-extern int ttm_write_lock(struct ttm_lock *lock, bool interruptible);
-
-/**
* ttm_lock_downgrade
*
* @lock: Pointer to a struct ttm_lock
diff --git a/sys/dev/drm2/ttm/ttm_page_alloc.c b/sys/dev/drm2/ttm/ttm_page_=
alloc.c
index ffc8483..9a30a46 100644
--- a/sys/dev/drm2/ttm/ttm_page_alloc.c
+++ b/sys/dev/drm2/ttm/ttm_page_alloc.c
@@ -113,16 +113,22 @@ struct ttm_pool_manager {
struct ttm_pool_opts options;
=20
union {
- struct ttm_page_pool pools[NUM_POOLS];
- struct {
- struct ttm_page_pool wc_pool;
- struct ttm_page_pool uc_pool;
- struct ttm_page_pool wc_pool_dma32;
- struct ttm_page_pool uc_pool_dma32;
- } ;
- };
+ struct ttm_page_pool u_pools[NUM_POOLS];
+ struct _utag {
+ struct ttm_page_pool u_wc_pool;
+ struct ttm_page_pool u_uc_pool;
+ struct ttm_page_pool u_wc_pool_dma32;
+ struct ttm_page_pool u_uc_pool_dma32;
+ } _ut;
+ } _u;
};
=20
+#define pools _u.u_pools
+#define wc_pool _u._ut.u_wc_pool
+#define uc_pool _u._ut.u_uc_pool
+#define wc_pool_dma32 _u._ut.u_wc_pool_dma32
+#define uc_pool_dma32 _u._ut.u_uc_pool_dma32
+
MALLOC_DEFINE(M_TTM_POOLMGR, "ttm_poolmgr", "TTM Pool Manager");
=20
static void
--jaoouwwPWoQSJZYp
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iQIbBAEBAgAGBQJRNhFAAAoJEJDCuSvBvK1BpWQP9AgDjnFoVqBbbiQVEvSgSXpn
KtZ6XfYhJL3aqAVlejKDmRNxOigU7t89bNxomYS+4BHxl8iTgX0O1KyBxi9muzKP
VrwtfpD4K5Q77pM8EkQRb3PscxUUO0KuJlluk+pZlfb4w/Wh1bBXLmBcfprU645J
RRTW6dm9D4Vkj/CmojqM0es2eI34hlJclqGkXAvez4a8PZIV4bzffoDF1DmehQnt
3QenI3M9fEwbg2CR1TYohsAgTZkEEJnuHlVjVB7Ll2xjr9aobrmTmRm9ZjUctTfL
PHSZjgc3gDAfdHtpovW7DMrpJ37umXxqSi5INXkUvPKoML0Ejz8HW+QiSksFH7uB
exwHeqKDGno4jKL625hvhLov3P3aK//50ZlWjW3E5ujqaLw0q48DEgJsACoEYu8e
R2m6MlOHqxuLtIaMLW93ghJxu2ihP/nzj7GkP2Nv15/QR9H76ADTT4MnYD9S1bUr
HU5S+/I+TibX2luIEtiokr4Y7Q6JC2GSl0NR/aqbXpuQOC+n8J6qifp/soXO/d+q
iLT7L6mM387gaGfw4aFZP2bTHD2Rzv5D5OEKp879Kwp9CCm7VdIlg9M4KcV8KILv
ZwYBoMch4T6GBnpykym0nXxXNa63J3CUf8c7Mn1zxqlAuPAg/vQuUnfSZlCQYjRM
uZk44OVKTRSabmxkWkI=
=hgsW
-----END PGP SIGNATURE-----
--jaoouwwPWoQSJZYp--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130305153736.GH3794>