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

[Stefan Richter <stefanr@s5r6.in-berlin.de>]

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).

So I am wondering, why would the barrier _not_ be needed here?

------------
 ¹) This was the original author's choice in his preference over using a
    lock for this access.  Another possibility for a lockless API would be
    to combine node_ID:generation into a single atomic variable; together
    they are 24 bits wide.
-- 
Stefan Richter
-=====-===== =--- -====
http://arcgraph.de/sr/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel