Re: Audio device mmap and kevents
At Sat, 19 Jan 2019 16:08:37 +0100, yarl-bau...@mailoo.org wrote: > > At first, mmap'ing audio device does not lead to improve any > > performance or latency. > From audio(4): > For audio applications that mmap(2) the audio device for play back > the > resultant latency is a third (1/3) of the value of the > hw.driverN.latency > variable. > > This is for the latency, but maybe the manual is outdated? I don't know about that but I find that hard to believe. > For the performance, while I'm not experienced, I think that mmap is > faster, less copies, no? mmap(2) may reduce copies between user space and the kernel compared to write(2). Generally speaking, yes. But your patch seems to increase memcpy(3) instead of decreasing write(2) system call. You need to consider whether the performance improvement is really necessary or not. And then if yes, you need to measure it before talking. This article is not for NetBSD but good to read about mmap. http://manuals.opensound.com/developer/mmap.html > > And it has to store the number of bytes writeable to kn->kn_data, > > not offset. > That's too bad but why not. See kqueue(4). audio should follow it even mmap case. EVFILT_WRITE Takes a descriptor as the identifier, and returns whenever it is possible to write to the descriptor. For sockets, pipes, fifos, and ttys, data will contain the amount of space remaining in the write buffer. And one more point: what do you do in such case? time1: Since kevent() wokes up, you were able to know the potision to write next in mmap area. time2: Copy next data to this position got in time1. time1.5: audio HW intterupt occurred. It advances the position in kernel. > > # Although I don't know whether it's the optimal way or not. > > And emulated flag has no meaning after 8.0. > From audio(4): > Only encodings that are not emulated (i.e. where > AUDIO_ENCODINGFLAG_EMULATED is not set) work properly for a mapped > device. Ah... The meaning of this flag was changed on 8.0. So this sentence itself about mmap seems not wrong. --- Tetsuya Isaki
Re: SIGBUS + coredump
On 19.01.2019 18:32, Jason Thorpe wrote: > > >> On Jan 19, 2019, at 8:55 AM, David Holland wrote: >> >> That's equivalent to writing out zeros (but more efficient) -- I would >> say the page shouldn't appear in the dump at all on the grounds that >> it doesn't actually exist. >> >> Whether it's ok to have an empty segment, I dunno. See if gdb chokes >> on it :-) > > Hm, then perhaps I'm misremembering how this is supposed to be represented in > the core. The issue is that there *is* a mapping at the virtual address, but > that fault can't be serviced. Making it appear as zeros or ZFOD gives the > wrong impression to the person doing the debugging. > > -- thorpej > I will skip these inaccessible segments (mappings) in a core(5) file generator. From a debugger point of view, skipping it is perhaps more verbose that it's not accessible. signature.asc Description: OpenPGP digital signature
re: Audio device mmap and kevents
i will have a look at the audioplay changes in the next week. .mrg.
Re: SIGBUS + coredump
> On Jan 19, 2019, at 8:55 AM, David Holland wrote: > > That's equivalent to writing out zeros (but more efficient) -- I would > say the page shouldn't appear in the dump at all on the grounds that > it doesn't actually exist. > > Whether it's ok to have an empty segment, I dunno. See if gdb chokes > on it :-) Hm, then perhaps I'm misremembering how this is supposed to be represented in the core. The issue is that there *is* a mapping at the virtual address, but that fault can't be serviced. Making it appear as zeros or ZFOD gives the wrong impression to the person doing the debugging. -- thorpej
Re: SIGBUS + coredump
On Fri, Jan 18, 2019 at 09:25:18AM -0800, Jason Thorpe wrote: > > > > On Jan 18, 2019, at 1:12 AM, Kamil Rytarowski wrote: > > > >> ISTM that it would better to skip the page than write out zeros... > >> > > > > This behavior is also fine. > > > > Should it be - in this example - a zero-sized segment? > > Yes, at the VA where the tail-hole exists, I think you want p_memsz > to reflect the size of the tail-hole, but p_filesz should be 0. That's equivalent to writing out zeros (but more efficient) -- I would say the page shouldn't appear in the dump at all on the grounds that it doesn't actually exist. Whether it's ok to have an empty segment, I dunno. See if gdb chokes on it :-) -- David A. Holland dholl...@netbsd.org
Re: Audio device mmap and kevents
At first, mmap'ing audio device does not lead to improve any performance or latency. From audio(4): For audio applications that mmap(2) the audio device for play back the resultant latency is a third (1/3) of the value of the hw.driverN.latency variable. This is for the latency, but maybe the manual is outdated? For the performance, while I'm not experienced, I think that mmap is faster, less copies, no? And I think that write(2) already has the optimal synchronization mechanism especially for stream-type data like audio. You don't have to monitor the kernel event. You don't have to calculate the circular buffer's offset. mmap is harder to use than write, this is usually the case but you get better performances. And it has to store the number of bytes writeable to kn->kn_data, not offset. That's too bad but why not. It is already implemented as "audioctl encodings" command. Ok, I didn't know. # Although I don't know whether it's the optimal way or not. And emulated flag has no meaning after 8.0. From audio(4): Only encodings that are not emulated (i.e. where AUDIO_ENCODINGFLAG_EMULATED is not set) work properly for a mapped device. Merging play() and play_fd() makes it harder to read. I do not agree.. It was necessary for me to add mmap support. Less redundancies and better structure, I think.
Re: Audio device mmap and kevents
It's better to use write(2) for this purpose.
At first, mmap'ing audio device does not lead to improve any
performance or latency. And I think that write(2) already has
the optimal synchronization mechanism especially for stream-type
data like audio. You don't have to monitor the kernel event.
You don't have to calculate the circular buffer's offset.
There are comments below.
At Fri, 18 Jan 2019 20:32:45 +0100,
yarl-bau...@mailoo.org wrote:
> Hello,
>
> I would like to propose a small improvement to the audio system.
> I think that it is very interesting to be able to mmap the audio device
> for better
> performance and smaller latency but It seems neither clean nor optimal
> to use
> AUDIO_GETOOFFS ioctl and sleep to synchronize.
>
> Why don't we use kernel events?
>
> Something like audio.patch for the kernel.
> I also implemented that use in audioplay and tried to clean a bit the
> code in audioplay.patch.
>
> There is certainly more work to be done but I have to ask now. What do
> you think?
>
> Thanks.
:
> --- a/sys/dev/audio.c
> +++ b/sys/dev/audio.c
> @@ -3463,11 +3466,21 @@ filt_audiowrite(struct knote *kn, long hint)
> mutex_enter(sc->sc_intr_lock);
>
> stream = chan->vc->sc_pustream;
> - kn->kn_data = (stream->end - stream->start)
> - - audio_stream_get_used(stream);
> + if (vc->sc_mpr.mmapped) {
> + off_t offset;
> + offset = stream->outp - stream->start
> + + vc->sc_mpr.blksize;
> + if (stream->start + offset >= stream->end)
> + offset = 0;
> + kn->kn_data = offset;
> + } else {
> + kn->kn_data = (stream->end - stream->start)
> + - audio_stream_get_used(stream);
> + rv = kn->kn_data > 0;
> + }
> mutex_exit(sc->sc_intr_lock);
>
> - return kn->kn_data > 0;
> + return rv;
> }
filt_audiowrite() has to return zero if it's not writeable.
And it has to store the number of bytes writeable to kn->kn_data,
not offset.
> --- a/usr.bin/audio/play/audioplay.1
> +++ b/usr.bin/audio/play/audioplay.1
> @@ -118,6 +118,12 @@ sample rate.
> Print a help message.
> .It Fl i
> If the audio device cannot be opened, exit now rather than wait for it.
> +.It Fl l
> +List the audio encodings supported by the device and if it emulated
> +or not. Useful to know what format to use when using
It is already implemented as "audioctl encodings" command.
# Although I don't know whether it's the optimal way or not.
And emulated flag has no meaning after 8.0.
> --- a/usr.bin/audio/play/play.c
> +++ b/usr.bin/audio/play/play.c
> -/*
> - * play the file on the file descriptor fd
> - */
> -static void
> -play_fd(const char *file, int fd)
> -{
Merging play() and play_fd() makes it harder to read.
Thanks.
---
Tetsuya Isaki
