Messages in this thread | | | From | "Altobelli, David" <> | Date | Thu, 12 Jun 2008 20:16:02 +0000 | Subject | RE: [PATCH] HP iLO driver |
| |
Heikki Orsila wrote: > On Thu, Jun 12, 2008 at 12:23:08PM -0600, David Altobelli wrote: >> + volatile u64 fifobar[1]; >> +}; > > Why do you need a volatile? What you probably want is atomic ops. > Spinlocks will create memory barriers implicitly.
This points to a queue that is shared with hardware, and could be modified outside of kernel control.
>> +static int fifo_enqueue(struct ilo_hwinfo *hw, char *fifobar, int >> entry) +{ + struct fifo *Q = FIFOBARTOHANDLE(fifobar); >> + int ret = 0; >> + >> + spin_lock(&hw->fifo_lock); >> + if (!(Q->fifobar[(Q->tail + 1) & Q->imask] & ENTRY_MASK_O)) { >> + Q->fifobar[Q->tail & Q->imask] |= >> + ((entry & ENTRY_MASK_NOSTATE) | Q->merge); + >> Q->tail += 1; + ret = 1; >> + } >> + spin_unlock(&hw->fifo_lock); >> + >> + return ret; >> +} > > Is writing to Q->fifobar (u64 *) endian-safe?
No, this is not endian-safe. Good point. I think converting these to readl() operations would let me remove the volatile and fix the endian issue.
| |