Re: device driver race conditions problems (kernel 2.4)

From: Paulo Garcia (paulo_at_digivoice.com.br)
Date: 08/28/04 

Date: 28 Aug 2004 05:50:49 -0700

Pete,

thanks for your answer. First at all, I'm new on linux device drivers,
then may be I was made the right choice on design. Any ideas will be
appreciated.

> > My driver must be able to manage more than one card using the same ISR
> > and putting all signals into the same sbuffer. My approach is to use
> > just one inode file (/dev/vbe1_s) to send data of all cards.
>
> Why have you decided to do it this way?

Is this a mistake? My usermode part is a shared object to allow
programmers to access the device driver through a simple API, then I
think it's more easy to do that using one file. Any suggestions?

> The code snippet below cannot work right, because the user context
> part is not interlocked with the interrupt processing. It has no
> calls to spin_lock_irqsave at all.

My cautions was to prevent "dead-lock" between to cards into the
interrupt handler. Because I used a circular buffer I was think that
isn't necessary to put spin_lock_irqsave on this function. My tests
with one card works well.

> However, if it did, it would be tricky. You cannot call copy_to_user
> while under lock, or you lock up. So, you would need to flip pointers
> under lock to extract a buffer, then copy it out.

I don't understand what you said about "to flip pointers". Can you
explain it?

> If you need real help, you will have to do better than this
> and perhaps post real code.

You are right. I was try to put only the main code to make a short
message. Below you have the real code.

Thanks in advance.

spinlock_t irq_lock = SPIN_LOCK_UNLOCKED;
//Interrupt Handle
void plx_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
 u32 tmp32;
 unsigned int buffer[1024];
          u16 addr_buffer;
          int nChannel;
          int addr_pci=0;
          int current_board;
        u8 num_cmds, loop_cmds;
          int bd = *(int*)dev_id; /* retrieves board index */
        unsigned long flags;

        
        spin_lock_irqsave(&irq_lock,flags);

        // First, verify if this interrupt belongs to the right card
        for (current_board=0;current_board<nCardsCount;current_board++)
        {
                tmp32=plx_read(card_array[current_board].addr_base1,PLX_INTCSR);
                if ((tmp32 & 0x04))
                {
                        //here i'm setting on this bit to trigger with oscilloscope
                        tmp32=plx_read(card_array[current_board].addr_base1,E1_CNTRL);
                        tmp32&=(~BIT26);
                        plx_write(card_array[current_board].addr_base1,tmp32,E1_CNTRL);
                        
                        //PLX9030 needs to set BIT2 of HPIC register
                        write_data(current_board,HPIC,0x000c); /* Sets directly */
                        break; //stops loop imediatelly
                }
        }
        
        //read commands from card
        //reads control data
        write_data(current_board,HPIA,CMD_CTR_DSP);
        tmp32 = read_data(current_board,HPID);
        //number of commands
        num_cmds = (u8)tmp32;
        if ((u8)(tmp32 & 0x8000))
        {
                 xprintk("Buffer Overflow!!!!\n");
        }
        if (num_cmds>0)
        {
                //there are commands then I need to read the active buffer
                addr_buffer = (tmp32 & BIT14);
                if (addr_buffer==0)
                        addr_buffer = CMD_DSP_HOST0;
                else
                        addr_buffer = CMD_DSP_HOST1;

                //get all data - my card use an DSP HPI port with autoinc to write
into memory card.
                write_data(current_board,HPIA,addr_buffer);
                if (num_cmds*CMD_SIZE_RX*sizeof(int) <= sizeof(buffer))
                        memcpy_fromio(buffer,card_array[current_board].mem_virt+HPID_AINC,num_cmds*CMD_SIZE_RX*sizeof(int));
                else
                        memcpy_fromio(buffer,card_array[current_board].mem_virt+HPID_AINC,sizeof(buffer));
                        
                //put into sbuffer
                for (loop_cmds=0; loop_cmds<(num_cmds*sizeof(int));loop_cmds=loop_cmds+4)
                {
                        nChannel = (u8)(buffer[loop_cmds+1]);
                        //Put it in the signal buffer
                        sbuffer.signal[sbuffer.wp] = (u8)(buffer[loop_cmds]);
                        if (nChannel < MAX_CHANNELS_CARD) //test port number
                                sbuffer.port[sbuffer.wp] = (u8)(nChannel +
(current_board*MAX_CHANNELS_CARD));
                        else
                                sbuffer.port[sbuffer.wp] = (u8)(nChannel);

                        sbuffer.data[sbuffer.wp] = (u8)(buffer[loop_cmds+2]);
                        //inc sbuffer pointer
                        inc_pointer_signal(&sbuffer.wp,1);
                }
                //wake up read function
                wake_up_interruptible(&sbuffer.signal_queue);

                //at finish - reset control byte to allow dsp to put more data
                write_data(current_board,HPIA,CMD_CTR_DSP);
                write_data(current_board,HPID,0);

        } //end read commands
        
        //here i'm setting off this bit to trigger with oscilloscope
        tmp32=plx_read(card_array[current_board].addr_base1,E1_CNTRL);
        tmp32|=BIT26;
        plx_write(card_array[current_board].addr_base1,tmp32,E1_CNTRL);
        
        spin_unlock_irqrestore(&irq_lock,flags);
}
//-------- end of isr
-------------------------------------------------------------------
        
//------------------- Read function part
------------------------------------------------
vppci_read (struct file *filp, char *buf, size_t count,loff_t *pos) //
{
  if(local_minor_number == 1) /* It's de _S files */
  {
  unsigned int s_remaning,d_remaning,transfered;
  if (wait_event_interruptible(sbuffer.signal_queue, (sbuffer.rp!=
sbuffer.wp)))
   return -ERESTARTSYS;

  if (sbuffer.rp!= sbuffer.wp)
  {
   s_remaning = copy_to_user(buf,&sbuffer.signal[sbuffer.rp],1);
   d_remaning = copy_to_user(buf+1,&sbuffer.data[sbuffer.rp],1);
   ret = copy_to_user(buf+2,&sbuffer.port[sbuffer.rp],1);
   transfered = 1 - s_remaning;
   inc_pointer_signal(&sbuffer.rp, transfered);
   return 1;
  }
 }
}

Relevant Pages