[alsa-devel] [PATCH 0/5 v2] ALSA: firewire-lib: purge restriction of synchronization for non-blocking mode

[Takashi Sakamoto]

On May 22 2015 23:00, Takashi Sakamoto wrote:
> This patchset renews my previous one.
> 
> [alsa-devel] [PATCH 0/4] firewire-lib: purge restriction of synchronization for non-blocking mode
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-May/092086.html
> 
> Changes:
>  * Use stack to keep calculation result per callback of isochronous receive context.
>  * Fix my stupid mistake to compare byte count to quadlet count. (oh...)
>  * Rename flag to proper name.
>  * Add 4th patch newly for better reading.
> 
> 
> Takashi Sakamoto (5):
>   ALSA: firewire-lib: add buffer-over-run protection at receiving more
>     data blocks than expected
>   ALSA: firewire-lib: simplify function to calculate the number of data
>     blocks
>   ALSA: firewire-lib: pass the number of data blocks in incoming packets
>     to outgoing packets
>   ALSA: firewire-lib: set streaming error outside of packetization
>   ALSA: firewire-lib: remove restriction for non-blocking mode
> 
>  sound/firewire/amdtp.c            | 151 ++++++++++++++++++++++++--------------
>  sound/firewire/amdtp.h            |   4 +
>  sound/firewire/oxfw/oxfw-stream.c |  10 ++-
>  3 files changed, 106 insertions(+), 59 deletions(-)

I realized that 3rd and 4th patches cause unexpected execution under
compiler optimization. As a result, when setting error state to incoming
stream, this module can cause invalid paging request.

I guess that 'struct amdtp_stream.packet_index' is set with minus value
and it's used to refer to packet buffer (struct
amdtp_stream.buffer.packets). Then a pointer of the buffer refers to
invalid page.

I tested -O1 and -O2 options for gcc. In both cases,
"handle_in_packet()" is merged into "in_stream_callback()", as I
expected. While, the error condition may not be handled as my
expectation. Especially under -O2, even if 'handle_in_packet()' returns
minus value, the loop in 'in_stream_callback()' doesn't finished till
all of packets are handled. In this case, when detecting packet
discontinuity, several error messages occur against my intension (one
iteration).

Briefly, I move 's->packet_index = -1' from 'in_stream_callback()' to
'handle_in_packet()' and the stack seems to work as I expected. But I'm
not so good at compiler optimization, so need a bit time to fix this issue..

Sorry to post patchset including bugs...


Regards

Takashi Sakamoto