On 12/01/2008, Andrew Morton <[EMAIL PROTECTED]> wrote: > On Sat, 12 Jan 2008 14:14:01 +0000 Adrian McMenamin <[EMAIL PROTECTED]> wrote: > > > > > > > + spin_command->cmd[0] = 0x70; > > > > + spin_command->cmd[2] = 0x1f; > > > > + spin_command->buflen = 0; > > > > + gd.pending = 1; > > > > + gdrom_packetcommand(gd.cd_info, spin_command); > > > > + /* 60 second timeout */ > > > > + wait_event_interruptible_timeout(command_queue, gd.pending == 0, HZ * > > > > 60); > > > > + gd.pending = 0; > > > > + kfree(spin_command); > > > > + if (gd.status & 0x01) { > > > > + /* log an error */ > > > > + gdrom_getsense(NULL); > > > > + return -EIO; > > > > + } > > > > + return 0; > > > > +} > > > > > > If the wait_event_interruptible_timeout() indeed times out, we go ahead > > > and > > > free spin_command. But someone else could potentially be using it. > > > > > > Suppose gdrom_packetcommand() got stuck for a minute due to bad hardware, > > > or some SCHED_FIFO task preempting us here and running for 61 seconds > > > without > > > yielding or something similarly weird. > > > > > > > > > Maybe I am being stupid here, but I don't follow this. They'll get a > > non-fatal error, that's all. Who else would be using spin_command? It's > > just a series of bytes to plug into the GD Rom registers, that's all. > > > > After programming the registers we need to wait for the interrupt to clear > gd.pending, don't we? > > <looks> > > oh, I see. gd is a global singleton and we only support one command at a > time and one device. hrm. >
OK, at the risk of looking like a fool: Yes, it does treat this as only one device per box as that's the way the things are made. The hardware will stop you from running two commands in parallel - the device will be flagged as busy and so you won't be able to get it do anything sensible. I have now added in some additional checks so that a user cannot start another command before the old one finishes - but I still don't see what the problem is with freeing the memory. The variable is a local, and an automatic, not a global or a static, so I don't see how kfree() could be releasing somebody else's lump of memory anyway. Or have I missed something obvious? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/