Date: Sun, 21 Aug 2016 21:32:35 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Slawa Olhovchenkov <slw@zxy.spb.ru> Cc: Ed Schouten <ed@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r304555 - head/sys/compat/cloudabi Message-ID: <20160821210751.J2219@besplex.bde.org> In-Reply-To: <20160821105207.GS22212@zxy.spb.ru> References: <201608210741.u7L7fBnN075023@repo.freebsd.org> <20160821105207.GS22212@zxy.spb.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160821210751.J2219>