[alsa-devel] [PATCH 03/25] ALSA: firewire-lib: add helper functions for asynchronous MIDI port

[Takashi Iwai]

On Sat, 15 Aug 2015 12:15:54 +0200,
Stefan Richter wrote:
> 
> On Aug 13 Takashi Sakamoto wrote:
> > On Aug 13 2015 15:31, Takashi Iwai wrote:
> > > On Thu, 13 Aug 2015 02:19:59 +0200,
> > > Takashi Sakamoto wrote:
> > >> --- a/sound/firewire/lib.c
> > >> +++ b/sound/firewire/lib.c
> [...]
> > >> +static void midi_port_tasklet(unsigned long data)
> > >> +{
> > >> +	struct snd_fw_async_midi_port *port =
> > >> +					(struct snd_fw_async_midi_port *)data;
> > >> +	struct snd_rawmidi_substream *substream = ACCESS_ONCE(port->substream);
> > >> +	int generation;
> > >> +	int type;
> [...]
> > >> +	/* Start this transaction. */
> > >> +	generation = port->parent->generation;
> > >> +	smp_rmb(); /* node_id vs. generation */
> > >
> > > Why this barrier is needed?
> > 
> > Oops. I forgot to refine it after copying from the other place. Exactly, 
> > it's not need here.
> > 
> > >> +	fw_send_request(port->parent->card, &port->transaction, type,
> > >> +			port->parent->node_id, generation,
> > >> +			port->parent->max_speed, port->addr,
> > >> +			port->buf, port->len, async_midi_port_callback,
> > >> +			port);
> > >> +}
> 
> The barrier is normally needed because:
>   - "generation" is the IEEE 1394 bus generation, which is a counter that
>     distinguishes periods between bus resets.
>   - At a bus reset, nodes may assume different node IDs from before the
>     reset.  Hence whenever a node is addressed by means of node ID, the
>     generation value must be given too.
>   - The node_ID:generation tuple is accessed in the Linux firewire stack in
>     a lockless manner¹:
>        - firewire-core's bus reset handler writes them by
>          set node_id; smp_wmb(); set generation
>        - higher-level code reads them by
>          get generation; smp_rmb(); get node_id
>     This way highlevel always only uses current IDs in current generations
>     or current IDs in outdated generations (the latter case being
>     extremely rare and doing no harm because the link layer hardware will
>     not emit this sort of requests to the bus), while on the other hand
>     highlevel never uses outdated IDs in current generations (which would
>     cause requests to be misdirected to a wrong node).

Thanks, that clarifies.  But wouldn't it be more helpful to have some
macro than open-coding at each place?


Takashi