From owner-svn-src-all@freebsd.org Sun Aug 21 11:32:48 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id DF75EBBF658; Sun, 21 Aug 2016 11:32:48 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 9E6F31D93; Sun, 21 Aug 2016 11:32:48 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c122-106-149-109.carlnfd1.nsw.optusnet.com.au (c122-106-149-109.carlnfd1.nsw.optusnet.com.au [122.106.149.109]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 0C55CD6454C; Sun, 21 Aug 2016 21:32:36 +1000 (AEST) Date: Sun, 21 Aug 2016 21:32:35 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Slawa Olhovchenkov cc: Ed Schouten , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r304555 - head/sys/compat/cloudabi In-Reply-To: <20160821105207.GS22212@zxy.spb.ru> Message-ID: <20160821210751.J2219@besplex.bde.org> References: <201608210741.u7L7fBnN075023@repo.freebsd.org> <20160821105207.GS22212@zxy.spb.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=CoZCCSMD c=1 sm=1 tr=0 a=R/f3m204ZbWUO/0rwPSMPw==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=CBc9DcqgLzKOA6Pp88gA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 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: Sun, 21 Aug 2016 11:32:49 -0000 On Sun, 21 Aug 2016, Slawa Olhovchenkov wrote: > On Sun, Aug 21, 2016 at 07:41:11AM +0000, Ed Schouten wrote: >> ... >> Log: >> Use memcpy() to copy 64-bit timestamps into the syscall return values. >> >> On 32-bit platforms, our 64-bit timestamps need to be split up across >> two registers. A simple assignment to td_retval[0] will cause the top 32 >> bits to get lost. By using memcpy(), we will automatically either use 1 >> or 2 registers depending on the size of register_t. >> .. >> Modified: head/sys/compat/cloudabi/cloudabi_clock.c >> ============================================================================== >> --- head/sys/compat/cloudabi/cloudabi_clock.c Sun Aug 21 07:28:38 2016 (r304554) >> +++ head/sys/compat/cloudabi/cloudabi_clock.c Sun Aug 21 07:41:11 2016 (r304555) >> @@ -117,7 +117,7 @@ cloudabi_sys_clock_res_get(struct thread >> error = cloudabi_convert_timespec(&ts, &cts); >> if (error != 0) >> return (error); >> - td->td_retval[0] = cts; >> + memcpy(td->td_retval, &cts, sizeof(cts)); >> return (0); >> } >> >> @@ -129,6 +129,6 @@ cloudabi_sys_clock_time_get(struct threa >> int error; >> >> error = cloudabi_clock_time_get(td, uap->clock_id, &ts); >> - td->td_retval[0] = ts; >> + memcpy(td->td_retval, &ts, sizeof(ts)); >> return (error); > > Why do not use more simple solution: Because it doesn't work. > *(cloudabi_timestamp_t *)td->td_retval = cts; > > This is eliminate call to memcpy and allow compiler to use most > effeicient way. memcpy() is the most efficient way (except for the kernel pessimization of losing builtin memcpy with -ffreestanding 15 years ago and not bringing it back). *(foo_t *)asks for alignment bugs. We have already fixed lots of these bugs for copying struct timevals in places like ping.c. Compilers warn about misalignment when certain warnings are enabled, but only on arches where misalignment is more than a pessimization. There is no reason why td_retval would be always aligned on these arches. Alignment of 64-bit types on 32-bit arches is usually so unimportant that even int32_t is not required to be aligned by the ABI, and there is no point in aligning td_retval specially unless you also do it for a large fraction of 64-bit integers in the kernel, and there are negative points for doing that. Bruce