From owner-svn-src-all@FreeBSD.ORG Wed Jan 28 17:02:36 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 17FA0F74; Wed, 28 Jan 2015 17:02:36 +0000 (UTC) Received: from st11p02mm-asmtp002.mac.com (st11p02mm-asmtpout002.mac.com [17.172.220.237]) (using TLSv1.2 with cipher DHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DAB19E0B; Wed, 28 Jan 2015 17:02:35 +0000 (UTC) Received: from [10.0.1.3] (unknown [73.162.13.215]) by st11p02mm-asmtp002.mac.com (Oracle Communications Messaging Server 7.0.5.35.0 64bit (built Dec 4 2014)) with ESMTPSA id <0NIW00JWJCNMSO20@st11p02mm-asmtp002.mac.com>; Wed, 28 Jan 2015 17:02:14 +0000 (GMT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.13.68,1.0.33,0.0.0000 definitions=2015-01-28_02:2015-01-28,2015-01-28,1970-01-01 signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1412080000 definitions=main-1501280172 Content-type: text/plain; charset=us-ascii MIME-version: 1.0 (Mac OS X Mail 8.1 \(1993\)) Subject: Re: svn commit: r277835 - in head: lib/libpmc sys/arm/arm sys/arm/include sys/arm/ti sys/conf sys/dev/hwpmc sys/sys From: Rui Paulo In-reply-to: <1422462903.15718.60.camel@freebsd.org> Date: Wed, 28 Jan 2015 09:02:10 -0800 Content-transfer-encoding: quoted-printable Message-id: <8F8272CF-DD73-41EF-8096-E8C79AD903CB@me.com> References: <201501281608.t0SG88gs009253@svn.freebsd.org> <1422462903.15718.60.camel@freebsd.org> To: Ian Lepore X-Mailer: Apple Mail (2.1993) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, Ruslan Bukin , src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Jan 2015 17:02:36 -0000 On Jan 28, 2015, at 08:35, Ian Lepore wrote: >=20 > On Wed, 2015-01-28 at 16:08 +0000, Ruslan Bukin wrote: >> Author: br >> Date: Wed Jan 28 16:08:07 2015 >> New Revision: 277835 >> URL: https://svnweb.freebsd.org/changeset/base/277835 >>=20 >> Log: >> Add ARMv7 performance monitoring counters. >>=20 >> Differential Revision: https://reviews.freebsd.org/D1687 >> Reviewed by: rpaulo >> Sponsored by: DARPA, AFRL >>=20 >> Added: >> head/sys/dev/hwpmc/hwpmc_armv7.c (contents, props changed) >> head/sys/dev/hwpmc/hwpmc_armv7.h (contents, props changed) >> Modified: >> head/lib/libpmc/libpmc.c >> head/sys/arm/arm/intr.c >> head/sys/arm/include/pmc_mdep.h >> head/sys/arm/ti/files.ti >> head/sys/conf/files.arm >> head/sys/dev/hwpmc/hwpmc_arm.c >> head/sys/dev/hwpmc/pmc_events.h >> head/sys/sys/pmc.h >=20 > This was in phabricator for review for 27 hours before it got = committed, > that's not enough time to allow people to actually review it. That > would be not enough time for even a simple change, let alone over a > thousand of lines of code. It certainly wasn't reviewed by those > actively working on arm pmc stuff recently (gnn and to a lesser = degree, > me). >=20 > Just from a quick glance at the part that wasn't truncated, I notice = all > the inline asm stuff is wrong -- it duplicates what's already = available > in cpu-v6.h. I do agree that reviewers weren't given enough time and phabricator = seems to make that worse by saying the code is "ready to land" (we're = trying to change that). In my defense, I only reviewed this because I implemented the original = XScale PMC. I also didn't know who else was working on ARM PMC, so I = couldn't warn Ruslan. Andrew was the one that added the ARM group to = the review, so maybe he wasn't aware of it either. -- Rui Paulo