| Message ID | 20170131115913.6828.12266.stgit@PASHA-ISP (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH] audio: make audio poll timer deterministic
Message-id: 20170131115913.6828.12266.stgit@PASHA-ISP
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20170131115913.6828.12266.stgit@PASHA-ISP -> patchew/20170131115913.6828.12266.stgit@PASHA-ISP
Switched to a new branch 'test'
9c7ac53 audio: make audio poll timer deterministic
=== OUTPUT BEGIN ===
Checking PATCH 1/1: audio: make audio poll timer deterministic...
ERROR: space prohibited between function name and open parenthesis '('
#23: FILE: audio/audio.c:1116:
+ timer_mod (s->ts,
total: 1 errors, 0 warnings, 12 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Hi On Tue, Jan 31, 2017 at 4:03 PM Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> wrote: > This patch changes resetting strategy of the audio polling timer. > It does not change expiration time if the timer is already set. > > You describe the change, but could you explain the reasons you propose it? > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > audio/audio.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index c845a44..1ee95a5 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void) > static void audio_reset_timer (AudioState *s) > { > if (audio_is_timer_needed ()) { > - timer_mod (s->ts, > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); > + if (!timer_pending(s->ts)) { > + timer_mod (s->ts, > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > conf.period.ticks); > + } > } > else { > timer_del (s->ts); > > > -- Marc-André Lureau
Ping? Pavel Dovgalyuk > -----Original Message----- > From: Pavel Dovgalyuk [mailto:pavel.dovgaluk@ispras.ru] > Sent: Tuesday, January 31, 2017 2:59 PM > To: qemu-devel@nongnu.org > Cc: pbonzini@redhat.com; dovgaluk@ispras.ru; kraxel@redhat.com > Subject: [PATCH] audio: make audio poll timer deterministic > > This patch changes resetting strategy of the audio polling timer. > It does not change expiration time if the timer is already set. > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > --- > audio/audio.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/audio/audio.c b/audio/audio.c > index c845a44..1ee95a5 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void) > static void audio_reset_timer (AudioState *s) > { > if (audio_is_timer_needed ()) { > - timer_mod (s->ts, > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); > + if (!timer_pending(s->ts)) { > + timer_mod (s->ts, > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); > + } > } > else { > timer_del (s->ts);
Hi, It looks to me that this behavior can be achieved with "timer_mod_anticipate()" function instead of a separate check. On Sun, Feb 12, 2017 at 9:04 PM, Pavel Dovgalyuk <dovgaluk@ispras.ru> wrote: > Ping? > > Pavel Dovgalyuk > > > > -----Original Message----- > > From: Pavel Dovgalyuk [mailto:pavel.dovgaluk@ispras.ru] > > Sent: Tuesday, January 31, 2017 2:59 PM > > To: qemu-devel@nongnu.org > > Cc: pbonzini@redhat.com; dovgaluk@ispras.ru; kraxel@redhat.com > > Subject: [PATCH] audio: make audio poll timer deterministic > > > > This patch changes resetting strategy of the audio polling timer. > > It does not change expiration time if the timer is already set. > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> > > --- > > audio/audio.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/audio/audio.c b/audio/audio.c > > index c845a44..1ee95a5 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > > @@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void) > > static void audio_reset_timer (AudioState *s) > > { > > if (audio_is_timer_needed ()) { > > - timer_mod (s->ts, > > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); > > + if (!timer_pending(s->ts)) { > > + timer_mod (s->ts, > > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > conf.period.ticks); > > + } > > } > > else { > > timer_del (s->ts); > > > >
On 13/02/2017 19:40, Yurii Zubrytskyi wrote: > Hi, > > It looks to me that this behavior can be achieved with > "timer_mod_anticipate()" function instead of a separate check. True, but I think Pavel's version is more readable. Since the timer is set to a fixed value from QEMU_CLOCK_VIRTUAL, timer_mod_anticipate is never going to do anything if the timer is already set. Paolo > > diff --git a/audio/audio.c b/audio/audio.c > > index c845a44..1ee95a5 100644 > > --- a/audio/audio.c > > +++ b/audio/audio.c > > @@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void) > > static void audio_reset_timer (AudioState *s) > > { > > if (audio_is_timer_needed ()) { > > - timer_mod (s->ts, > > - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > conf.period.ticks); > > + if (!timer_pending(s->ts)) { > > + timer_mod (s->ts, > > + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + > conf.period.ticks); > > + } > > } > > else { > > timer_del (s->ts); > > > > > > > -- > Thanks, Yurii
diff --git a/audio/audio.c b/audio/audio.c index c845a44..1ee95a5 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1112,8 +1112,10 @@ static int audio_is_timer_needed (void) static void audio_reset_timer (AudioState *s) { if (audio_is_timer_needed ()) { - timer_mod (s->ts, - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); + if (!timer_pending(s->ts)) { + timer_mod (s->ts, + qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks); + } } else { timer_del (s->ts);
This patch changes resetting strategy of the audio polling timer. It does not change expiration time if the timer is already set. Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru> --- audio/audio.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)