Date: Sun, 08 May 2011 14:50:54 +0200 From: Andreas Tobler <andreast@FreeBSD.org> To: Attilio Rao <attilio@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Nathan Whitehorn <nwhitehorn@FreeBSD.org>, andreast@FreeBSD.org Subject: Re: svn commit: r221550 - head/sys/powerpc/conf Message-ID: <4DC691AE.701@FreeBSD.org> In-Reply-To: <BANLkTimNC4rZvvaH5Dk%2BQpNgE90BjoQ-Sg@mail.gmail.com> References: <201105062043.p46Kh2Vs065320@svn.freebsd.org> <BANLkTimiKGWeJfdCQrcPvujdfuyvMX8wbQ@mail.gmail.com> <BANLkTimNC4rZvvaH5Dk%2BQpNgE90BjoQ-Sg@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------020806010604040907000603 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 08.05.11 00:17, Attilio Rao wrote: > 2011/5/6 Attilio Rao<attilio@freebsd.org>: >> 2011/5/6 Nathan Whitehorn<nwhitehorn@freebsd.org>: >>> Author: nwhitehorn >>> Date: Fri May 6 20:43:02 2011 >>> New Revision: 221550 >>> URL: http://svn.freebsd.org/changeset/base/221550 >>> >>> Log: >>> SMP has worked perfectly for a very long time on 32-bit PowerPC on both >>> UP and SMP hardware. Enable it in GENERIC. >>> >> >> While working on largeSMP, I think there is a breakage in atomic.h. >> More specifically, atomic_store_rel_long() (and related functions) are >> not going to properly work because powerpc defines them as: >> >> atomic_store_rel_long -> atomic_store_rel_32(volatile u_int *p, u_int v) >> >> while this should really follow the long arguments. >> >> This happens because powerpc doesn't follow the other architectures >> semantic on defining the "similar" atomic operations. >> Other arches define an hardcode version of _type version of the >> function and than make a macro the _32 (or whatever) version. >> In other words this is what they do: >> >> void >> atomic_store_rel_32() >> { >> ... >> } >> >> #define atomic_store_rel_int atomic_store_rel_32 >> >> which si clearly dangerous for cases as reported above. Maybe that >> could be fixed by passing sized types, rather than simply int or long >> in numbered version, but I'd really prefer to follow the semantic by >> other architectures and then have: >> >> void >> atomic_store_rel_int() >> { >> ... >> } >> >> #define atomic_store_rel_32 atomic_store_rel_int >> >> I fixed the ATOMIC_STORE_LOAD case in my code, because I needed it, >> but the final cleanup is much bigger. >> I can make a patch tomorrow if you can test it. >> > > Can you please test and review this patch?: > http://www.freebsd.org/~attilio/largeSMP/atomic-powerpc.diff > > Unfortunately I'm having issues with the toolchains in atm, so I can't > really neither test compile it. I built kernel and world on both, on a 32-bit system and on a 64-bit system with 32-bit compat support. The 32-bit world failed due to type punning issues from umtx.h:121ff Attached my try to workaround these issues. With my try I can build both, kernel and world. Right now I have a world running with it (32-bit). I do not know if my try is correct. Even less after reading the comments from Bruce. But I think if it is on a somehow correct way, then we need something similar for the other _long functions on 32-bit where we go from the u_long to u_int. (e.g, atomic_add_long etc.) I'm ready for more testing. Gruss, Andreas --------------020806010604040907000603 Content-Type: text/plain; x-mac-type="0"; x-mac-creator="0"; name="atomic_ppc-1.diff" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="atomic_ppc-1.diff" LS0tIGF0b21pYy5oLmF0dGlsaW8JMjAxMS0wNS0wOCAxMTowMDo0Mi4wMDAwMDAwMDAgKzAy MDAKKysrIGF0b21pYy5oLmFuZHJlYXMJMjAxMS0wNS0wOCAxNDowMTowMC4wMDAwMDAwMDAg KzAyMDAKQEAgLTY1MSwxNiArNjUxLDI3IEBACiAjZGVmaW5lCWF0b21pY19jbXBzZXRfcmVs X3B0cihkc3QsIG9sZCwgbmV3KQkJCQlcCiAJYXRvbWljX2NtcHNldF9yZWxfbG9uZygodm9s YXRpbGUgdV9sb25nICopKGRzdCksICh1X2xvbmcpKG9sZCksCVwKIAkgICAgKHVfbG9uZyko bmV3KSkKLSNlbHNlCi0jZGVmaW5lCWF0b21pY19jbXBzZXRfbG9uZyhkc3QsIG9sZCwgbmV3 KQkJCQlcCi0JYXRvbWljX2NtcHNldF9pbnQoKHZvbGF0aWxlIHVfaW50ICopKGRzdCksICh1 X2ludCkob2xkKSwJXAotCSAgICAodV9pbnQpKG5ldykpCi0jZGVmaW5lCWF0b21pY19jbXBz ZXRfYWNxX2xvbmcoZHN0LCBvbGQsIG5ldykJCQkJXAotCWF0b21pY19jbXBzZXRfYWNxX2lu dCgodm9sYXRpbGUgdV9pbnQgKikoZHN0KSwgKHVfaW50KShvbGQpLAlcCi0JICAgICh1X2lu dCkobmV3KSkKLSNkZWZpbmUJYXRvbWljX2NtcHNldF9yZWxfbG9uZyhkc3QsIG9sZCwgbmV3 KQkJCQlcCi0JYXRvbWljX2NtcHNldF9yZWxfaW50KCh2b2xhdGlsZSB1X2ludCAqKShkc3Qp LCAodV9pbnQpKG9sZCksCVwKLQkgICAgKHVfaW50KShuZXcpKQorI2Vsc2UgIC8qIF9fcG93 ZXJwYzY0X18gICovCitzdGF0aWMgX19pbmxpbmUgaW50CithdG9taWNfY21wc2V0X2xvbmco dm9sYXRpbGUgdV9sb25nICpkc3QsIHVfbG9uZyBvbGQsIHVfbG9uZyBuZXcpCit7CisJcmV0 dXJuIChhdG9taWNfY21wc2V0X2ludCgodm9sYXRpbGUgdV9pbnQgKilkc3QsICh1X2ludClv bGQsIAorCQkJCSAgKHVfaW50KW5ldykpOworfQorCitzdGF0aWMgX19pbmxpbmUgaW50Cith dG9taWNfY21wc2V0X2FjcV9sb25nKHZvbGF0aWxlIHVfbG9uZyAqZHN0LCB1X2xvbmcgb2xk LCB1X2xvbmcgbmV3KQoreworCXJldHVybiAoYXRvbWljX2NtcHNldF9hY3FfaW50KCh2b2xh dGlsZSB1X2ludCAqKWRzdCwgKHVfaW50KW9sZCwgCisJCQkJICAgICAgKHVfaW50KW5ldykp OworfQorCitzdGF0aWMgX19pbmxpbmUgaW50CithdG9taWNfY21wc2V0X3JlbF9sb25nKHZv bGF0aWxlIHVfbG9uZyAqZHN0LCB1X2xvbmcgb2xkLCB1X2xvbmcgbmV3KQoreworCXJldHVy biAoYXRvbWljX2NtcHNldF9yZWxfaW50KCh2b2xhdGlsZSB1X2ludCAqKWRzdCwgKHVfaW50 KW9sZCwgCisJCQkJICAgICAgKHVfaW50KW5ldykpOworfQogCiAjZGVmaW5lCWF0b21pY19j bXBzZXRfcHRyKGRzdCwgb2xkLCBuZXcpCQkJCVwKIAlhdG9taWNfY21wc2V0X2ludCgodm9s YXRpbGUgdV9pbnQgKikoZHN0KSwgKHVfaW50KShvbGQpLAlcCkBAIC02NzEsNyArNjgyLDcg QEAKICNkZWZpbmUJYXRvbWljX2NtcHNldF9yZWxfcHRyKGRzdCwgb2xkLCBuZXcpCQkJCVwK IAlhdG9taWNfY21wc2V0X3JlbF9pbnQoKHZvbGF0aWxlIHVfaW50ICopKGRzdCksICh1X2lu dCkob2xkKSwJXAogCSAgICAodV9pbnQpKG5ldykpCi0jZW5kaWYKKyNlbmRpZiAgLyogX19w b3dlcnBjNjRfXyAgKi8KIAogc3RhdGljIF9faW5saW5lIHVfaW50CiBhdG9taWNfZmV0Y2hh ZGRfaW50KHZvbGF0aWxlIHVfaW50ICpwLCB1X2ludCB2KQo= --------------020806010604040907000603--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4DC691AE.701>