From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 98C25C433B4 for ; Sat, 10 Apr 2021 11:42:46 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C56BE610F7 for ; Sat, 10 Apr 2021 11:42:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C56BE610F7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=diwic.se Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id BAB2715E0; Sat, 10 Apr 2021 13:41:52 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz BAB2715E0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1618054962; bh=qV9k4hji4k9aLRTIyTnGyGL2AfRtbEHHbXiwHX2tDXk=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=S2pDHMg+EUmDLSvAaviACpWoQ5Bz9lFt2D/8TGHoPfCYNXy1ADLvvbOQNL/jDN+J5 BhYRgAAWiZrZZpeOLgE07UvOtXyMwzhpbdKu1nqRIJa9HZOE29pSSuE2LaefKhGx4x w//mRXrJUlwyIwo/IXecl/Y+ZomJTn8IkBhtOlvM= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id 3A121F80169; Sat, 10 Apr 2021 13:41:52 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 2D2F7F8020B; Sat, 10 Apr 2021 13:41:51 +0200 (CEST) Received: from ns4.inleed.net (mailout4.inleed.net [IPv6:2a0b:dc80:cafe:104::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id D7547F800BC for ; Sat, 10 Apr 2021 13:41:47 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz D7547F800BC Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=diwic.se header.i=@diwic.se header.b="XBDobhPp" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=diwic.se; s=x; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Sender:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=MqfHPPlp4LUDAs2eJgmSouwd5eMPkV0TDFvZvALZuBE=; b=XBDobhPpS+WsmyAdJ40SKjt/3F RYnDm2xKhzpneeTiQHpQsGHBHSJzSmun4IXLF0hC8uTcPQvhVBlbZlBfbfmYO9k9n3frCLeRKaAd9 JFZZ8MfDGXtzaRDQEz5oWc2OCsgu2nBgEQfLMnrqYyTMLGZdTA4nwYCgS4ca/1OFo1bRBJaAX/0uq UWxEyetUp3v/LGA0v+AtXqy1bDx3gTtg2PuAvaYKEpoXOl9UMnq0KzqwhZS8nFqfqyIEhbO26iA3h vW034A4K1RRUr3bbB+nmC0SpQOMkRBUxWt+5F/5uyy8GRIxxJEM9tUAa1Ls78hrONfKTqhD4xQ2um OggiuWqw==; Received: from c83-254-143-147.bredband.comhem.se ([83.254.143.147] helo=[192.168.5.7]) by ns4.inleed.net with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94) (envelope-from ) id 1lVBzn-00Di2f-FN; Sat, 10 Apr 2021 13:41:47 +0200 Subject: Re: [PATCH v2] sound: rawmidi: Add framing mode To: Takashi Iwai References: <20210324054253.34642-1-coding@diwic.se> <20210324124430.GA3711@workstation> <057ef387-9ee1-2678-29ce-d644f2a3a90a@diwic.se> <20210326044615.GA51246@workstation> <2ca71809-9872-bfee-c19d-76b6ce143212@diwic.se> From: David Henningsson Message-ID: Date: Sat, 10 Apr 2021 13:41:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Authenticated-Id: coding@diwic.se Cc: alsa-devel@alsa-project.org X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On 2021-04-06 14:01, Takashi Iwai wrote: > On Mon, 05 Apr 2021 14:13:27 +0200, > David Henningsson wrote: >> >> On 2021-03-31 09:40, Takashi Iwai wrote: >>> On Tue, 30 Mar 2021 21:35:11 +0200, >>> David Henningsson wrote: >>>> Well, I believe that rawmidi provides less jitter than seq is not a >>>> theoretical problem but a known fact (see e g [1]), so I haven't tried >>>> to "prove" it myself. And I cannot read your mind well enough to know >>>> what you would consider a sufficient proof - are you expecting to see >>>> differences on a default or RT kernel, on a Threadripper or a >>>> Beaglebone, idle system or while running RT torture tests? Etc. >>> There is certainly difference, and it might be interesting to see the >>> dependency on the hardware or on the configuration. But, again, my >>> primary question is: have you measured how *your patch* really >>> provides the improvement? If yes, please show the numbers in the >>> patch description. >> As you requested, I have now performed such testing. >> >> Results: >> >> Seq - idle: 5.0 ms >> >> Seq - hackbench: 1.3 s (yes, above one second) >> >> Raw + framing - idle: 2.8 ms >> >> Raw + framing - hackbench: 2.8 ms >> >> Setup / test description: >> >> I had an external midi sequencer connected through USB. The system >> under test was a Celeron N3150 with internal graphics. The sequencer >> was set to generate note on/note off commands exactly 10 times per >> second. >> >> For the seq tests I used "arecordmidi" and analyzed the delta values >> of resulting midi file. For the raw + framing tests I used a home-made >> application to write a midi file. The monotonic clock option was used >> to rule out differences between monotonic and monotonic_raw. The >> result shown above is the maximum amount of delta value, converted to >> milliseconds, minus the expected 100 ms between notes. Each test was >> run for a minute or two. >> >> For the "idle" test, the machine was idle (running a normal desktop), >> and for the "hackbench" test, "chrt -r 10 hackbench" was run a few >> times in parallel with the midi recording application (which was run >> with "chrt -r 15"). >> >> I also tried a few other stress tests but hackbench was the one that >> stood out as totally destroying the timestamps of seq midi. (E g, >> running "rt-migrate-test" in parallel with "arecordmidi" gave a max >> jitter value of 13 ms.) >> >> Conclusion: >> >> I still believe the proposed raw + framing mode is a valuable >> improvement in the normal/idle case, but even more so because it is >> more stable in stressed conditions. Do you agree? > Thanks for the tests. Yes, that's an interesting and convincing > result. > > Could you do a couple of favors in addition? Okay, now done. Enjoy :-) > > 1) Check the other workqueue > > It's interesting to see whether the hiprio system workqueue may give a > better latency. A oneliner patch is like below. > > -- 8< -- > --- a/sound/core/rawmidi.c > +++ b/sound/core/rawmidi.c > @@ -1028,7 +1028,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, > } > if (result > 0) { > if (runtime->event) > - schedule_work(&runtime->event_work); > + queue_work(system_highpri_wq, &runtime->event_work); > else if (__snd_rawmidi_ready(runtime)) > wake_up(&runtime->sleep); > } > -- 8< -- Result: idle: 5.0 ms hackbench > 1 s I e, same as original. > > Also, system_unbound_wq can be another interesting test case instead > of system_highpri_wq. > > 2) Direct sequencer event process > > If a chance of workqueue doesn't give significant improvement, we > might need to check the direct invocation of the sequencer > dispatcher. A totally untested patch is like below. > > -- 8< -- > --- a/sound/core/rawmidi.c > +++ b/sound/core/rawmidi.c > @@ -979,6 +979,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, > unsigned long flags; > int result = 0, count1; > struct snd_rawmidi_runtime *runtime = substream->runtime; > + bool call_event = false; > > if (!substream->opened) > return -EBADFD; > @@ -1028,11 +1029,13 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, > } > if (result > 0) { > if (runtime->event) > - schedule_work(&runtime->event_work); > + call_event = true; > else if (__snd_rawmidi_ready(runtime)) > wake_up(&runtime->sleep); > } > spin_unlock_irqrestore(&runtime->lock, flags); > + if (call_event) > + runtime->event(runtime->substream); > return result; > } > EXPORT_SYMBOL(snd_rawmidi_receive); > > -- 8< -- Result: Idle: 3.0 ms Hackbench still > 1s. The reason that this is 3.0 and not 2.8 is probably during to some rounding to whole ms somewhere in either seq or arecordmidi - I'd say this is likely the same 2.8 ms as we see from the rawmidi+framing test. > In theory, this should bring to the same level of latency as the > rawmidi timestamping. Of course, this doesn't mean we can go straight > to this way, but it's some material for consideration. I don't know why the hackbench test is not improved here. But you seem to have changed seq from tasklet to workqueue in 2011 (commit b3c705aa9e9), presumably for some relevant reason, like a need to sleep in the seq code...? // David