All of lore.kernel.org
 help / color / mirror / Atom feed
* asihpi: Need help converting volatile to memory barriers
@ 2008-02-21 23:18 Eliot Blennerhassett
  2008-02-22 10:44 ` Clemens Ladisch
  2008-02-22 17:37 ` Greg KH
  0 siblings, 2 replies; 3+ messages in thread
From: Eliot Blennerhassett @ 2008-02-21 23:18 UTC (permalink / raw)
  To: alsa-devel; +Cc: Takashi Iwai, greg

Greetings,

first, the meta-help request:
Is there another place that I should be making this request? (lkml, kernel 
newbies, linux driver project etc)?

Theres quite a bit written about barriers, but most seems to be assuming SMP 
situation or memory mapped devices. Not much about devices doing DMA.
I.e I have read Documentation/memory-barriers.txt

Now the actual question:
Currently I have a driver that uses "volatile" - heres the relevant source.
http://hg.alsa-project.org/alsa-driver/file/89222d702376/pci/asihpi/hpi6205.c

The "volatile" is applied to structures that are either read or written by 
device DMA.  Certainly the driver in its current state doesn't work without 
volatile qualifier. (BTW the device doesn't use host interrupts)

Structures ("interface") used for dma are allocated with dma_alloc_coherent()

In the following, am I using the barriers correctly?

1) Reading something updated by DMA

volatile struct bus_master_interface *interface;
while (interface->ack != OK) { 
	sleep(a while)  	
	[ device changes interface->ack by dma ]
};
=== after conversion
struct bus_master_interface *interface;
while (interface->ack != OK) { 
	sleep(a while);
	rmb(); 
};

Here the volatile or rmb is needed or the loop gets optimised away.

2) Writing to memory, interrupt device

volatile struct bus_master_interface *interface;
interface->cmd = command;
iowrite(device_interrupt, 1);

=== after conversion ===
struct bus_master_interface *interface;
interface->cmd = command;
wmb();
iowrite(device_interrupt, 1);
[device reads interface->cmd by dma]

Is the wmb() a guarantee that the command will be in memory visible to the 
device when the driver informs it of a new command?


3) 

One assumption I am making is that the compiler is not going to optimise 
across functions 
E.g. in the following scenario, is the compiler going to optimise the loop 
away without a rmb()?  If not, is this because of something inherent in the C 
standard, or just because the optimiser isn't yet smart enough to see it? 
I.e. it might work now, but when whole-file-optimisation is introduced, it 
will fail?

int get_ack(interface) { return interface->ack }
...
while (get_ack(interface) != OK) {
	sleep(a while);
}


regards

--
Eliot Blennerhassett
www.audioscience.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: asihpi: Need help converting volatile to memory barriers
  2008-02-21 23:18 asihpi: Need help converting volatile to memory barriers Eliot Blennerhassett
@ 2008-02-22 10:44 ` Clemens Ladisch
  2008-02-22 17:37 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Clemens Ladisch @ 2008-02-22 10:44 UTC (permalink / raw)
  To: Eliot Blennerhassett; +Cc: alsa-devel

Eliot Blennerhassett wrote:
> Theres quite a bit written about barriers, but most seems to be assuming SMP
> situation or memory mapped devices. Not much about devices doing DMA.
> I.e I have read Documentation/memory-barriers.txt

See also <Documentation/DMA-mapping.txt>.

> interface->cmd = command;
> wmb();
> iowrite(device_interrupt, 1);
> [device reads interface->cmd by dma]
>
> Is the wmb() a guarantee that the command will be in memory visible to the
> device when the driver informs it of a new command?

Yes.  Please not that accesses to I/O space have an implicit barrier, so
you don't even need the wmb() in this case.

> One assumption I am making is that the compiler is not going to optimise
> across functions
> E.g. in the following scenario, is the compiler going to optimise the loop
> away without a rmb()?  If not, is this because of something inherent in the C
> standard, or just because the optimiser isn't yet smart enough to see it?
> I.e. it might work now, but when whole-file-optimisation is introduced, it
> will fail?
>
> int get_ack(interface) { return interface->ack }
> ...
> while (get_ack(interface) != OK) {
> 	sleep(a while);
> }

The get_ack() function is an obvious candidate for inlining.  The C
standard definitely allows it to be optimized away (if the compiler can
prove that sleep() doesn't write to interface->ack).  Both gcc an icc
are able to do (or plan to introduce) this optimization.


HTH
Clemens

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: asihpi: Need help converting volatile to memory barriers
  2008-02-21 23:18 asihpi: Need help converting volatile to memory barriers Eliot Blennerhassett
  2008-02-22 10:44 ` Clemens Ladisch
@ 2008-02-22 17:37 ` Greg KH
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2008-02-22 17:37 UTC (permalink / raw)
  To: Eliot Blennerhassett; +Cc: Takashi Iwai, alsa-devel

On Fri, Feb 22, 2008 at 12:18:18PM +1300, Eliot Blennerhassett wrote:
> Greetings,
> 
> first, the meta-help request:
> Is there another place that I should be making this request? (lkml, kernel 
> newbies, linux driver project etc)?

lkml is best for this.

> Theres quite a bit written about barriers, but most seems to be assuming SMP 
> situation or memory mapped devices. Not much about devices doing DMA.
> I.e I have read Documentation/memory-barriers.txt
> 
> Now the actual question:
> Currently I have a driver that uses "volatile" - heres the relevant source.
> http://hg.alsa-project.org/alsa-driver/file/89222d702376/pci/asihpi/hpi6205.c
> 
> The "volatile" is applied to structures that are either read or written by 
> device DMA.  Certainly the driver in its current state doesn't work without 
> volatile qualifier. (BTW the device doesn't use host interrupts)
> 
> Structures ("interface") used for dma are allocated with dma_alloc_coherent()
> 
> In the following, am I using the barriers correctly?
> 
> 1) Reading something updated by DMA
> 
> volatile struct bus_master_interface *interface;
> while (interface->ack != OK) { 
> 	sleep(a while)  	
> 	[ device changes interface->ack by dma ]
> };
> === after conversion
> struct bus_master_interface *interface;
> while (interface->ack != OK) { 
> 	sleep(a while);
> 	rmb(); 
> };
> 
> Here the volatile or rmb is needed or the loop gets optimised away.

Using rmb() is correct, not volatile, as volatile might not really do
what you are expecting it to do on all versions of gcc and
architectures.

So please remove all instances of volatile, it is not correct to use it
within kernel code.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-02-22 17:38 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-21 23:18 asihpi: Need help converting volatile to memory barriers Eliot Blennerhassett
2008-02-22 10:44 ` Clemens Ladisch
2008-02-22 17:37 ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.