From patchwork Mon Mar 3 15:38:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Lynch X-Patchwork-Id: 3755041 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B5966BF13A for ; Mon, 3 Mar 2014 15:39:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9B9A5203E3 for ; Mon, 3 Mar 2014 15:39:03 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 46E76203DF for ; Mon, 3 Mar 2014 15:39:02 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKUxE-0002H8-FR; Mon, 03 Mar 2014 15:38:56 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKUxB-0003Cg-Q8; Mon, 03 Mar 2014 15:38:53 +0000 Received: from relay1.mentorg.com ([192.94.38.131]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WKUx9-0003C4-D6 for linux-arm-kernel@lists.infradead.org; Mon, 03 Mar 2014 15:38:52 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1WKUwh-0003FZ-JM from Nathan_Lynch@mentor.com ; Mon, 03 Mar 2014 07:38:23 -0800 Received: from SVR-ORW-FEM-03.mgc.mentorg.com ([147.34.97.39]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Mon, 3 Mar 2014 07:38:23 -0800 Received: from localhost.localdomain (147.34.91.1) by SVR-ORW-FEM-03.mgc.mentorg.com (147.34.97.39) with Microsoft SMTP Server (TLS) id 14.2.247.3; Mon, 3 Mar 2014 07:38:22 -0800 Message-ID: <5314A1ED.10005@mentor.com> Date: Mon, 3 Mar 2014 09:38:21 -0600 From: Nathan Lynch User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Will Deacon Subject: Re: [PATCH v3] ARM: vDSO gettimeofday using generic timer architecture References: <1393813295-27376-1-git-send-email-nathan_lynch@mentor.com> <20140303110001.GB8554@mudshark.cambridge.arm.com> In-Reply-To: <20140303110001.GB8554@mudshark.cambridge.arm.com> X-OriginalArrivalTime: 03 Mar 2014 15:38:23.0464 (UTC) FILETIME=[9CA04E80:01CF36F6] X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140303_103851_576134_14EEC91F X-CRM114-Status: GOOD ( 23.46 ) X-Spam-Score: -1.9 (-) Cc: "steve.capper@linaro.org" , "linux@arm.linux.org.uk" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Will, On 03/03/2014 05:00 AM, Will Deacon wrote: > On Mon, Mar 03, 2014 at 02:21:35AM +0000, Nathan Lynch wrote: >> - Force reload of seq_count when spinning: without a memory clobber >> after the load of vdata->seq_count, GCC can generate code like this: >> 2f8: e59c9020 ldr r9, [ip, #32] >> 2fc: e3190001 tst r9, #1 >> 300: 1a000033 bne 3d4 >> 304: f57ff05b dmb ish >> 308: e59c3034 ldr r3, [ip, #52] ; 0x34 >> ... >> 3d4: eafffffe b 3d4 > > Have you thought about using a seqlock instead? It looks like a drop-in > replacement for your seqcnt_* stuff. I have considered that, and x86_64 in fact uses the seqlock APIs for its vDSO. I've made the change locally and it passes my tests fine, but I have one concern -- seqcount_t is defined as: typedef struct seqcount { unsigned sequence; #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map; #endif } seqcount_t; If we use a seqcount_t in the vdso_data structure and CONFIG_DEBUG_LOCK_ALLOC=y, not only does this bloat the structure to no purpose (we can't use the lockdep-enabled read accessors in userspace), it also leaks kernel pointer values and such through the lockdep_map member. So I think the seqcount would have to be a plain integer, and we'd have to cast to seqcount_t to manipulate it, e.g. seq = raw_read_seqcount_begin((seqcount_t *)&vdata->seq); See the incremental patch (damaged by my mailer, sorry) below for how this would look. } @@ -79,10 +58,10 @@ static int do_realtime_coarse(struct timespec *ts, struct vdso_data *vdata) static int do_monotonic_coarse(struct timespec *ts, struct vdso_data *vdata) { struct timespec tomono; - u32 seq; + unsigned long seq; do { - seq = seqcnt_acquire(vdata); + seq = raw_read_seqcount_begin((seqcount_t *)&vdata->seq); ts->tv_sec = vdata->xtime_coarse_sec; ts->tv_nsec = vdata->xtime_coarse_nsec; @@ -90,7 +69,7 @@ static int do_monotonic_coarse(struct timespec *ts, struct vdso_data *vdata) tomono.tv_sec = vdata->wtm_clock_sec; tomono.tv_nsec = vdata->wtm_clock_nsec; - } while (seq != seqcnt_read(vdata)); + } while (read_seqcount_retry((seqcount_t *)&vdata->seq, seq)); ts->tv_sec += tomono.tv_sec; timespec_add_ns(ts, tomono.tv_nsec); @@ -119,10 +98,10 @@ static u64 get_ns(struct vdso_data *vdata) static int do_realtime(struct timespec *ts, struct vdso_data *vdata) { u64 nsecs; - u32 seq; + unsigned long seq; do { - seq = seqcnt_acquire(vdata); + seq = raw_read_seqcount_begin((seqcount_t *)&vdata->seq); if (vdata->use_syscall) return -1; @@ -130,7 +109,7 @@ static int do_realtime(struct timespec *ts, struct vdso_data *vdata) ts->tv_sec = vdata->xtime_clock_sec; nsecs = get_ns(vdata); - } while (seq != seqcnt_read(vdata)); + } while (read_seqcount_retry((seqcount_t *)&vdata->seq, seq)); ts->tv_nsec = 0; timespec_add_ns(ts, nsecs); @@ -142,10 +121,10 @@ static int do_monotonic(struct timespec *ts, struct vdso_data *vdata) { struct timespec tomono; u64 nsecs; - u32 seq; + unsigned long seq; do { - seq = seqcnt_acquire(vdata); + seq = raw_read_seqcount_begin((seqcount_t *)&vdata->seq); if (vdata->use_syscall) return -1; @@ -156,7 +135,7 @@ static int do_monotonic(struct timespec *ts, struct vdso_data *vdata) tomono.tv_sec = vdata->wtm_clock_sec; tomono.tv_nsec = vdata->wtm_clock_nsec; - } while (seq != seqcnt_read(vdata)); + } while (read_seqcount_retry((seqcount_t *)&vdata->seq, seq)); ts->tv_sec += tomono.tv_sec; ts->tv_nsec = 0; diff --git a/arch/arm/include/asm/vdso_datapage.h b/arch/arm/include/asm/vdso_datapage.h index 9011ec75a24b..83a752f969da 100644 --- a/arch/arm/include/asm/vdso_datapage.h +++ b/arch/arm/include/asm/vdso_datapage.h @@ -27,7 +27,7 @@ * 32 bytes. */ struct vdso_data { - u32 seq_count; /* sequence count - odd during updates */ + unsigned long seq; /* sequence counter */ u16 use_syscall; /* whether to fall back to syscalls */ u16 cs_shift; /* clocksource shift */ u32 xtime_coarse_sec; /* coarse time */ diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c index 3db74240a766..6ff2893feb45 100644 --- a/arch/arm/kernel/vdso.c +++ b/arch/arm/kernel/vdso.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -109,18 +110,8 @@ void arm_install_vdso(struct mm_struct *mm) /** * update_vsyscall - update the vdso data page * - * Increment the sequence counter, making it odd, indicating to - * userspace that an update is in progress. Update the fields used - * for coarse clocks and, if the architected system timer is in use, - * the fields used for high precision clocks. Increment the sequence - * counter again, making it even, indicating to userspace that the - * update is finished. - * - * Userspace is expected to sample seq_count before reading any other - * fields from the data page. If seq_count is odd, userspace is - * expected to wait until it becomes even. After copying data from - * the page, userspace must sample seq_count again; if it has changed - * from its previous value, userspace must retry the whole sequence. + * Update the fields used for coarse clocks and, if the architected + * system timer is in use, the fields used for high precision clocks. * * Calls to update_vsyscall are serialized by the timekeeping core. */ @@ -130,8 +121,11 @@ void update_vsyscall(struct timekeeper *tk) struct timespec *wtm = &tk->wall_to_monotonic; bool use_syscall = strcmp(tk->clock->name, "arch_sys_counter"); - ++vdso_data->seq_count; - smp_wmb(); + BUILD_BUG_ON(offsetof(struct seqcount, sequence) != 0); + BUILD_BUG_ON(FIELD_SIZEOF(struct vdso_data, seq) != + FIELD_SIZEOF(struct seqcount, sequence)); + + raw_write_seqcount_begin((seqcount_t *)&vdso_data->seq); xtime_coarse = __current_kernel_time(); vdso_data->use_syscall = use_syscall; @@ -149,8 +143,7 @@ void update_vsyscall(struct timekeeper *tk) vdso_data->cs_mask = tk->clock->mask; } - smp_wmb(); - ++vdso_data->seq_count; + raw_write_seqcount_end((seqcount_t *)&vdso_data->seq); flush_dcache_page(virt_to_page(vdso_data)); } diff --git a/arch/arm/kernel/vdso/vgettimeofday.c b/arch/arm/kernel/vdso/vgettimeofday.c index d5d12528eed9..e434769817d3 100644 --- a/arch/arm/kernel/vdso/vgettimeofday.c +++ b/arch/arm/kernel/vdso/vgettimeofday.c @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -23,28 +24,6 @@ extern struct vdso_data *__get_datapage(void); -static u32 seqcnt_acquire(struct vdso_data *vdata) -{ - u32 seq; - - do { - seq = vdata->seq_count; - - /* Force gcc to reload from memory when spinning. */ - asm volatile("" : : : "memory"); - } while (seq & 1); - - dmb(ish); - return seq; -} - -static u32 seqcnt_read(struct vdso_data *vdata) -{ - dmb(ish); - - return ACCESS_ONCE(vdata->seq_count); -} - static long clock_gettime_fallback(clockid_t _clkid, struct timespec *_ts) { register struct timespec *ts asm("r1") = _ts; @@ -63,15 +42,15 @@ static long clock_gettime_fallback(clockid_t _clkid, struct timespec *_ts) static int do_realtime_coarse(struct timespec *ts, struct vdso_data *vdata) { - u32 seq; + unsigned long seq; do { - seq = seqcnt_acquire(vdata); + seq = raw_read_seqcount_begin((seqcount_t *)&vdata->seq); ts->tv_sec = vdata->xtime_coarse_sec; ts->tv_nsec = vdata->xtime_coarse_nsec; - } while (seq != seqcnt_read(vdata)); + } while (read_seqcount_retry((seqcount_t *)&vdata->seq, seq)); return 0;