1. 14 6月, 2017 1 次提交
  2. 12 6月, 2017 6 次提交
    • T
      ALSA: pcm: use %s instead of %c for format of PCM buffer tracepoints · f5abd532
      Takashi Sakamoto 提交于
      As long as I know, in userspace, '%c' format on printing format for
      tracepoint is replaced with '>c<' by existent tracing program; i.g.
      'perf-trace' and 'trace-cmd'. This is inconvenient.
      
      This commit replaces the format with '%s'. The length of letters in the
      format string is not changed, thus this commit doesn't increase object
      size.
      
      In theory, I should work for improvements of these tracing programs, but
      here I'd like to save my time to work for the other projects.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      f5abd532
    • T
      ALSA: pcm: add 'applptr' event of tracepoint · fccf5388
      Takashi Sakamoto 提交于
      In design of ALSA PCM core, status and control data for runtime of ALSA
      PCM substream are shared between kernel/user spaces by page frame
      mapping with read-only attribute. Both of hardware-side and
      application-side position on PCM buffer are maintained as a part of
      the status data. In a view of ALSA PCM application, these two positions
      can be updated by executing ioctl(2) with some commands.
      
      There's an event of tracepoint for hardware-side position; 'hwptr'.
      On the other hand, no events for application-side position. This commit
      adds a new event for this purpose; 'applptr'. When the application-side
      position is changed in kernel space, this event is probed with useful
      information for developers.
      
      I note that the event is not probed for all of ALSA PCM applications, When
      applications are written by read/write programming scenario, the event is
      surely probed. The applications execute ioctl(2) with
      SNDRV_PCM_IOCTL_[READ|WRITE][N/I]_FRAMES to read/write any PCM frame, then
      ALSA PCM core updates the application-side position in kernel land.
      However, when applications are written by mmap programming scenario, if
      maintaining the application side position in kernel space accurately,
      applications should voluntarily execute ioctl(2) with
      SNDRV_PCM_IOCTL_SYNC_PTR to commit the number of handled PCM frames. If
      not voluntarily, the application-side position is not changed, thus the
      added event is not probed.
      
      There's a loophole, using architectures to which ALSA PCM core judges
      non cache coherent. In this case, the status and control data is not mapped
      into processe's VMA for any applications. Userland library, alsa-lib, is
      programmed for this case. It executes ioctl(2) with
      SNDRV_PCM_IOCTL_SYNC_PTR command every time to requiring the status and
      control data.
      
      ARM is such an architecture. Below is an example with serial sound interface
      (ssi) on i.mx6 quad core SoC. I use v4.1 kernel released by fsl-community
      with patches from VIA Tech. Inc. for VAB820, and my backport patches for
      relevant features for this patchset. I use Ubuntu 17.04 from
      ports.ubuntu.com as user land for armhf architecture.
      
      $ aplay -v -M -D hw:imx6vab820sgtl5,0 /dev/urandom -f S16_LE -r 48000 --period-size=128 --buffer-size=256
      Playing raw data '/dev/urandom' : Signed 16 bit Little Endian, Rate 48000 Hz, Mono
      Hardware PCM card 0 'imx6-vab820-sgtl5000' device 0 subdevice 0
      Its setup is:
        stream       : PLAYBACK
        access       : MMAP_INTERLEAVED
        format       : S16_LE
        subformat    : STD
        channels     : 1
        rate         : 48000
        exact rate   : 48000 (48000/1)
        msbits       : 16
        buffer_size  : 256
        period_size  : 128
        period_time  : 2666
        tstamp_mode  : NONE
        tstamp_type  : MONOTONIC
        period_step  : 1
        avail_min    : 128
        period_event : 0
        start_threshold  : 256
        stop_threshold   : 256
        silence_threshold: 0
        silence_size : 0
        boundary     : 1073741824
        appl_ptr     : 0
        hw_ptr       : 0
      mmap_area[0] = 0x76f98000,0,16 (16)
      
      $ trace-cmd record -e snd_pcm:hwptr -e snd_pcm:applptr
      $ trace-cmd report
      ...
      60.208495: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256
      60.208633: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256
      60.210022: hwptr:   pcmC0D0p/sub0: IRQ: pos=128, old=1536, base=1536, period=128, buf=256
      60.210202: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256
      60.210344: hwptr:   pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256
      60.210348: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256
      60.210486: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256
      60.210626: applptr: pcmC0D0p/sub0: prev=1792, curr=1920, avail=0, period=128, buf=256
      60.211002: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256
      60.211142: hwptr:   pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256
      60.211146: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256
      60.211287: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256
      60.212690: hwptr:   pcmC0D0p/sub0: IRQ: pos=0, old=1664, base=1536, period=128, buf=256
      60.212866: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256
      60.212999: hwptr:   pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256
      60.213003: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256
      60.213135: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256
      60.213276: applptr: pcmC0D0p/sub0: prev=1920, curr=2048, avail=0, period=128, buf=256
      60.213654: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256
      60.213796: hwptr:   pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256
      60.213800: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256
      60.213937: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256
      60.215356: hwptr:   pcmC0D0p/sub0: IRQ: pos=128, old=1792, base=1792, period=128, buf=256
      60.215542: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256
      60.215679: hwptr:   pcmC0D0p/sub0: POS: pos=128, old=1920, base=1792, period=128, buf=256
      60.215683: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256
      60.215813: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256
      60.215947: applptr: pcmC0D0p/sub0: prev=2048, curr=2176, avail=0, period=128, buf=256
      ...
      
      We can surely see 'applptr' event is probed even if the application run
      for mmap programming scenario ('-M' option and 'hw' plugin). Below is a
      result of strace:
      
      02:44:15.886382 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.887203 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}])
      02:44:15.887471 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.887637 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.887805 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.887969 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.888132 read(3, "..."..., 256) = 256
      02:44:15.889040 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.889221 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.889431 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.889606 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}])
      02:44:15.889833 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.889998 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.890164 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.891048 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.891228 read(3, "..."..., 256) = 256
      02:44:15.891497 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.891661 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.891829 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      02:44:15.891991 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}])
      02:44:15.893007 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
      
      We can see 7 calls of ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR per loop with
      call of poll(2). 128 PCM frames are transferred per loop of one poll(2),
      because the PCM substream is configured with S16_LE format and 1 channel
      (2 byte * 1 * 128 = 256 bytes). This equals to the size of period of PCM
      buffer. Comparing to the probed data, one of the 7 calls of ioctl(2) is
      actually used to commit the number of copied PCM frames to kernel space.
      The other calls are just used to check runtime status of PCM substream;
      e.g. XRUN.
      
      The tracepoint event is useful to investigate this case. I note that below
      modules are related to the above sample.
      
       * snd-soc-dummy.ko
       * snd-soc-imx-sgtl5000.ko
       * snd-soc-fsl-ssi.ko
       * snd-soc-imx-pcm-dma.ko
       * snd-soc-sgtl5000.ko
      
      My additional note is lock acquisition. The event is probed under acquiring
      PCM stream lock. This means that calculation in the event is free from
      any hardware events.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      fccf5388
    • T
      ALSA: pcm: unify codes to operate application-side position on PCM buffer · 66e01a5c
      Takashi Sakamoto 提交于
      In a series of recent work, ALSA PCM core got some arrangements to handle
      application-side position on PCM buffer. However, relevant codes still
      disperse to two translation units
      
      This commit unifies these codes into a helper function.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      66e01a5c
    • T
      ALSA: seq: Allow the modular sequencer registration · 111b0cdb
      Takashi Iwai 提交于
      Many drivers bind the sequencer stuff in off-load by another driver
      module, so that it's loaded only on demand.  In the current code, this
      mechanism doesn't work when the driver is built-in while the sequencer
      is module.  We check with IS_REACHABLE() and enable only when the
      sequencer is in the same level of build.
      
      However, this is basically a overshoot.  The binder code
      (snd-seq-device) is an individual module from the sequencer core
      (snd-seq), and we just have to make the former a built-in while
      keeping the latter a module for allowing the scenario like the above.
      
      This patch achieves that by rewriting Kconfig slightly.  Now, a driver
      that provides the manual sequencer device binding should select
      CONFIG_SND_SEQ_DEVICE in a way as
      	select SND_SEQ_DEVICE if SND_SEQUENCER != n
      
      Note that the "!=n" is needed here to avoid the influence of the
      sequencer core is module while the driver is built-in.
      
      Also, since rawmidi.o may be linked with snd_seq_device.o when
      built-in, we have to shuffle the code to make the linker happy.
      (the kernel linker isn't smart enough yet to handle such a case.)
      That is, snd_seq_device.c is moved to sound/core from sound/core/seq,
      as well as Makefile.
      
      Last but not least, the patch replaces the code using IS_REACHABLE()
      with IS_ENABLED(), since now the condition meets always when enabled.
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      111b0cdb
    • T
      ALSA: pcm: probe events when parameters are changed actually · 82e7d501
      Takashi Sakamoto 提交于
      At present, trace events are probed even if corresponding parameter is
      not actually changed. This is inconvenient.
      
      This commit improves the behaviour.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      82e7d501
    • T
      ALSA: pcm: return error immediately for parameters handling · f74ae15f
      Takashi Sakamoto 提交于
      When refining mask/interval parameters, helper functions can return error
      code. This error is not handled immediately. This seems to return
      parameters to userspace applications in its meddle of processing.
      
      However, in general, when receiving error from system calls, the
      application might not handle argument buffer. It's reasonable to
      judge the above design as superfluity.
      
      This commit handles the error immediately.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      f74ae15f
  3. 10 6月, 2017 2 次提交
    • T
      ALSA: seq: Reorganize kconfig and build · 0181307a
      Takashi Iwai 提交于
      This is a slightly intensive rewrite of Kconfig and Makefile about
      ALSA sequencer stuff.
      
      The first major change is that the kconfig items for the sequencer are
      moved to sound/core/seq/Kconfig.  OK, that's easy.
      
      The substantial change is that, instead of hackish top-level module
      selection in Makefile, we define a Kconfig item for each sequencer
      module.  The driver that requires such sequencer components select
      exclusively the kconfig items.  This is more straightforward and
      standard way.
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      0181307a
    • T
      ALSA: seq: Allow the tristate build of OSS emulation · 3d774d5e
      Takashi Iwai 提交于
      Currently OSS sequencer emulation is tied with ALSA sequencer core,
      both are built in the same level; i.e. when CONFIG_SND_SEQUENCER=y,
      the OSS sequencer emulation is also always built-in, even though the
      functionality can be built as an individual module.
      
      This patch changes the rule and allows users to build snd-seq-oss
      module while others are built-in.  Essentially, it's just a few simple
      changes in Kconfig and Makefile.  Some driver codes like opl3 need to
      convert from the simple ifdef to IS_ENABLED().  But that's all.
      
      You might wonder how about the dependency: right, it can be messy, but
      it still works.  Since we rewrote the sequencer binding with the
      standard bus, the driver can be bound at any time on demand.  So, the
      synthesizer driver module can be loaded individually from the OSS
      emulation core before/after it.
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      3d774d5e
  4. 09 6月, 2017 15 次提交
  5. 07 6月, 2017 7 次提交
    • T
      ALSA: pcm: obsolete RULES_DEBUG local macro · c6706de0
      Takashi Sakamoto 提交于
      Added tracepoints obsoleted RULES_DEBUG local macro and relevant codes.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      c6706de0
    • T
      ALSA: pcm: enable parameter tracepoints only when CONFIG_SND_DEBUG is enabled · 37567c55
      Takashi Sakamoto 提交于
      In a previous commit, tracepoints are added for PCM parameter processing.
      As long as I know, this implementation increases size of relocatable
      object by 35%. For vendors who are conscious of memory footprint, it
      brings apparent disadvantage.
      
      This commit utilizes CONFIG_SND_DEBUG configuration to enable/disable the
      tracepoints.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      37567c55
    • T
      ALSA: pcm: tracepoints for refining PCM parameters · be4e31da
      Takashi Sakamoto 提交于
      When working for devices which support configurable modes for its data
      transmission or which consists of several components, developers are
      likely to use rules of parameters of PCM substream. However, there's no
      infrastructure to assist their work.
      
      In old days, ALSA PCM core got a local 'RULES_DEBUG' macro to debug
      refinement of parameters for PCM substream. Although this is merely a
      makeshift. With some modifications, we get the infrastructure.
      
      This commit is for the purpose. Refinement of mask/interval type of
      PCM parameters is probed as tracepoint events as 'hw_mask_param' and
      'hw_interval_param' on existent 'snd_pcm' subsystem.
      Signed-off-by: NTakashi Sakamoto <o-takashi@sakamocchi.jp>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      be4e31da
    • T
      ALSA: timer: Wrap with spinlock for queue access · d7f910bf
      Takashi Iwai 提交于
      For accessing the snd_timer_user queue indices, we take tu->qlock.
      But it's forgotten in a couple of places.
      
      The one in snd_timer_user_params() should be safe without the
      spinlock as the timer is already stopped.  But it's better for
      consistency.
      
      The one in poll is just a read-out, so it's not inevitably needed, but
      it'd be good to make the result consistent, too.
      Tested-by: NAlexander Potapenko <glider@google.com>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      d7f910bf
    • T
      ALSA: timer: Improve user queue reallocation · 890e2cb5
      Takashi Iwai 提交于
      ALSA timer may reallocate the user queue upon request, and it happens
      at three places for now: at opening, at SNDRV_TIMER_IOCTL_PARAMS, and
      at SNDRV_TIMER_IOCTL_SELECT.  However, the last one,
      snd_timer_user_tselect(), doesn't need to reallocate the buffer since
      it doesn't change the queue size.  It does just because tu->tread
      might have been changed before starting the timer.
      
      Instead of *_SELECT ioctl, we should reallocate the queue at
      SNDRV_TIMER_IOCTL_TREAD; then the timer is guaranteed to be stopped,
      thus we can reassign the buffer more safely.
      
      This patch implements that with a slight code refactoring.
      Essentially, the patch achieves:
      - Introduce realloc_user_queue() for (re-)allocating the ring buffer,
        and call it from all places.  Also, realloc_user_queue() uses
        kcalloc() for avoiding possible leaks.
      - Add the buffer reallocation at SNDRV_TIMER_IOCTL_TREAD.  When it
        fails, tu->tread is restored to the old value, too.
      - Drop the buffer reallocation at snd_timer_user_tselect().
      Tested-by: NAlexander Potapenko <glider@google.com>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      890e2cb5
    • T
      ALSA: timer: Fix missing queue indices reset at SNDRV_TIMER_IOCTL_SELECT · ba3021b2
      Takashi Iwai 提交于
      snd_timer_user_tselect() reallocates the queue buffer dynamically, but
      it forgot to reset its indices.  Since the read may happen
      concurrently with ioctl and snd_timer_user_tselect() allocates the
      buffer via kmalloc(), this may lead to the leak of uninitialized
      kernel-space data, as spotted via KMSAN:
      
        BUG: KMSAN: use of unitialized memory in snd_timer_user_read+0x6c4/0xa10
        CPU: 0 PID: 1037 Comm: probe Not tainted 4.11.0-rc5+ #2739
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
        Call Trace:
         __dump_stack lib/dump_stack.c:16
         dump_stack+0x143/0x1b0 lib/dump_stack.c:52
         kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1007
         kmsan_check_memory+0xc2/0x140 mm/kmsan/kmsan.c:1086
         copy_to_user ./arch/x86/include/asm/uaccess.h:725
         snd_timer_user_read+0x6c4/0xa10 sound/core/timer.c:2004
         do_loop_readv_writev fs/read_write.c:716
         __do_readv_writev+0x94c/0x1380 fs/read_write.c:864
         do_readv_writev fs/read_write.c:894
         vfs_readv fs/read_write.c:908
         do_readv+0x52a/0x5d0 fs/read_write.c:934
         SYSC_readv+0xb6/0xd0 fs/read_write.c:1021
         SyS_readv+0x87/0xb0 fs/read_write.c:1018
      
      This patch adds the missing reset of queue indices.  Together with the
      previous fix for the ioctl/read race, we cover the whole problem.
      Reported-by: NAlexander Potapenko <glider@google.com>
      Tested-by: NAlexander Potapenko <glider@google.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      ba3021b2
    • T
      ALSA: timer: Fix race between read and ioctl · d11662f4
      Takashi Iwai 提交于
      The read from ALSA timer device, the function snd_timer_user_tread(),
      may access to an uninitialized struct snd_timer_user fields when the
      read is concurrently performed while the ioctl like
      snd_timer_user_tselect() is invoked.  We have already fixed the races
      among ioctls via a mutex, but we seem to have forgotten the race
      between read vs ioctl.
      
      This patch simply applies (more exactly extends the already applied
      range of) tu->ioctl_lock in snd_timer_user_tread() for closing the
      race window.
      Reported-by: NAlexander Potapenko <glider@google.com>
      Tested-by: NAlexander Potapenko <glider@google.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: NTakashi Iwai <tiwai@suse.de>
      d11662f4
  6. 03 6月, 2017 9 次提交