On Jan 23, 2013, at 9:55 AM, Leif Lindholm <leif.lindh...@arm.com> wrote:
> To summarize the out-of-line assembler changes of this patch: > - The patch is functionally correct for ARMv7 (which we know, because the code > - It also appears to be functionally correct for ARMv6, given reports of > - It *might* be functionally correct for ARMv5, although I have seen no > - It is not functionally correct on ARMv4. Thanks for the summary (snipped, above). > [snip] > Basic point is - this is an insufficiently validated patch referred to as > "an ugly kludge" by the original author (Jon Masters@Red Hat), who created > it to be able to include it in the Fedora ARMv5 port. I has previously > provided suggestions for improvements, but it has still been submitted to > the Open MPI users list without any of those suggestions being acted on. > > I admit to being slightly miffed with it being accepted and applied without > ever being mentioned on the Open MPI developers list It was done by one of the core committers (George); it's in our community's culture to go commit without discussion on the devel list for many kinds of things. FWIW: Since we all know each other pretty well, we do a lot of communication via IM and telephone in addition to the public mailing list discussions. This is not because we're discussing secret things -- it's just that you can get a lot more accomplished in a 10 minute phone call than 15 back-n-forth, 10-page, highly detailed emails. > - only on the users list. All the above being said: you're absolutely right. We have not been careful about what gets discussed on the users' list vs. the devel list. You're right that this was discussed over on the users' list (because of a bug report; the conversation turned to devel-like topics, but stayed on the users' list). George committed a fix and then said "how's this?" (on the users' list). And he didn't consult you, the primary maintainer of this section of code. > A list to which I now find myself subscribed to without having asked > for or being told about - miffed again. Sorry about that; this was my fault. I interpreted your off-post mails to me about not being able to post to the users list as an ask to be subscribed (since we don't allow posts from unsubscribed users). Rather than unsubscribe you, though, I just marked you as "nomail" on the users' list. So you won't receive any further mail from that list, but you're still subscribed, so you can post. > If the main purpose of accepting this patch is to provide a stopgap measure > for something better, I would much prefer simply incorporating your > CCASFLAGS > workaround into the configure script - removing the out-of-line asm > implementations of the atomics, but still providing a functional library for > the most common use-cases. > > Something like: I tested this patch in v1.6 and v1.7 on my Pi, and it seems to work just fine. "make check" passes all the ASM tests. To be clear: I consider you to be the primary author and maintainer of this code, and you're certainly more of an ARM expert than any of us. George may not have realized that someone from ARM was still an active part of the community; I'm not sure. But I, too, vote that we should back out his changes from the trunk and put your suggested patch (his patch did not make it over to v1.6 or v1.7, because I was waiting for your response). We actually do try to get consensus for these kinds of things, so let's give George a little time to respond before backing it out. -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/
