Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1407835 > unrolled thread

[PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags Minfei Huang <mnghuan@gmail.com> - 2016-05-27 08:20 +0200
    Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get  variable flags Andy Lutomirski <luto@amacapital.net> - 2016-05-27 17:50 +0200
      Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get  variable flags Paolo Bonzini <pbonzini@redhat.com> - 2016-05-27 18:10 +0200
        Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get  variable flags Minfei Huang <mnghuan@gmail.com> - 2016-05-28 14:10 +0200
    [PATCH 3/3 V2] pvclock: Get rid of __pvclock_read_cycles in function pvclock_read_flags Minfei Huang <mnghuan@gmail.com> - 2016-05-28 14:30 +0200

#1407835 — [PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags

There is a generic function __pvclock_read_cycles to be used to get both
flags and cycles. For function pvclock_read_flags, it's useless to get
cycles value. To make this function be more effective, add a new wrapper
function to only get variable flags.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
---
 arch/x86/include/asm/pvclock.h | 12 ++++++++++++
 arch/x86/kernel/pvclock.c      |  3 +--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index 7c1c895..3c4e5b7 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -88,6 +88,18 @@ unsigned __pvclock_read_cycles(const struct pvclock_vcpu_time_info *src,
 	return version;
 }
 
+static __always_inline unsigned int __pvclock_read_flags(
+		const struct pvclock_vcpu_time_info *src, u8 *flags)
+{
+	unsigned int version;
+
+	version = src->version;
+	/* Make the latest version visible */
+	smp_rmb();
+	*flags = src->flags;
+	return version;
+}
+
 struct pvclock_vsyscall_time_info {
 	struct pvclock_vcpu_time_info pvti;
 } __attribute__((__aligned__(SMP_CACHE_BYTES)));
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 7f82fe0..138772c 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -61,11 +61,10 @@ void pvclock_resume(void)
 u8 pvclock_read_flags(struct pvclock_vcpu_time_info *src)
 {
 	unsigned version;
-	cycle_t ret;
 	u8 flags;
 
 	do {
-		version = __pvclock_read_cycles(src, &ret, &flags);
+		version = __pvclock_read_flags(src, &flags);
 		/* Make sure that the version double-check is last. */
 		smp_rmb();
 	} while ((src->version & 1) || version != src->version);
-- 
2.6.3

[toc] | [next] | [standalone]

#1408146 — Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags

On Thu, May 26, 2016 at 11:17 PM, Minfei Huang <mnghuan@gmail.com> wrote:
> There is a generic function __pvclock_read_cycles to be used to get both
> flags and cycles. For function pvclock_read_flags, it's useless to get
> cycles value. To make this function be more effective, add a new wrapper
> function to only get variable flags.

Why not just get rid of this function entirely?

[toc] | [prev] | [next] | [standalone]

#1408160 — Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags

On 27/05/2016 17:40, Andy Lutomirski wrote:
> On Thu, May 26, 2016 at 11:17 PM, Minfei Huang <mnghuan@gmail.com> wrote:
>> There is a generic function __pvclock_read_cycles to be used to get both
>> flags and cycles. For function pvclock_read_flags, it's useless to get
>> cycles value. To make this function be more effective, add a new wrapper
>> function to only get variable flags.
> 
> Why not just get rid of this function entirely?

I agree.  If you want to abstract the retry logic, perhaps you can add
pvclock_read_{begin,retry} functions like seqlock_read_{begin,retry}?

Thanks,

Paolo

[toc] | [prev] | [next] | [standalone]

#1408450 — Re: [PATCH 3/3] pvclock: Add a new wrapper function to only get variable flags

On 05/27/16 at 06:06P, Paolo Bonzini wrote:
> 
> 
> On 27/05/2016 17:40, Andy Lutomirski wrote:
> > On Thu, May 26, 2016 at 11:17 PM, Minfei Huang <mnghuan@gmail.com> wrote:
> >> There is a generic function __pvclock_read_cycles to be used to get both
> >> flags and cycles. For function pvclock_read_flags, it's useless to get
> >> cycles value. To make this function be more effective, add a new wrapper
> >> function to only get variable flags.
> > 
> > Why not just get rid of this function entirely?
> 
> I agree.  If you want to abstract the retry logic, perhaps you can add

Hmm, I will get rid of pvclock_read_flags, and repost it.

Thanks
Minfei

> pvclock_read_{begin,retry} functions like seqlock_read_{begin,retry}?
> 
> Thanks,
> 
> Paolo

[toc] | [prev] | [next] | [standalone]

#1408462 — [PATCH 3/3 V2] pvclock: Get rid of __pvclock_read_cycles in function pvclock_read_flags

There is a generic function __pvclock_read_cycles to be used to get both
flags and cycles. For function pvclock_read_flags, it's useless to get
cycles value. To make this function be more effective, get this variable
flags directly in function.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
---
v1:
- Get rid of __pvclock_read_cycles according to Andy's suggestion
---
 arch/x86/kernel/pvclock.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 7f82fe0..06c58ce 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -61,11 +61,14 @@ void pvclock_resume(void)
 u8 pvclock_read_flags(struct pvclock_vcpu_time_info *src)
 {
 	unsigned version;
-	cycle_t ret;
 	u8 flags;
 
 	do {
-		version = __pvclock_read_cycles(src, &ret, &flags);
+		version = src->version;
+		/* Make the latest version visible */
+		smp_rmb();
+
+		flags = src->flags;
 		/* Make sure that the version double-check is last. */
 		smp_rmb();
 	} while ((src->version & 1) || version != src->version);
-- 
2.6.3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web