Re: [PATCH] 3/3 maple: update bus driver to support Dreamcast VMU

[Adrian McMenamin]

On Mon, 2008-03-24 at 18:38 +0100, JÃrn Engel wrote:
> On Mon, 24 March 2008 18:07:07 +0100, JÃrn Engel wrote:
> > 
> > The second rearranges the list locking a bit.  Previously it was
> > possible to touch maple_waitq or maple_sentq without holding the lock.
> > With my limited understanding of the driver, the second patch may
> > already be enough to prevent the type of corruption you've been seeing.
> 
> Limited understanding indeed.  The problem in the mtd driver is that it
> has no concept of ownership.  For example maple_vmu_read_block() does
> the following:
> 	mdev->mq->sendbuf = sendbuf;
> 	...
> 	maple_add_packet(mdev->mq);
> 
> It accesses some field in mdev->mq, then sends the structure off to
> maple_add_packet().  From this point on, mdev->mq belongs to the bus
> driver - until it calls the callback, vmu_blockread() in this case.
> 
> But a second thread can call into maple_vmu_read_block() again and
> access mdev->mq while it rightfully belongs to the bus driver.  Bad
> things will happen.
> 
> So these two patches try to close the race windows.  Please note the
> FIXME in the write patch - I didn't do all the work.  Real life calls
> for attention, so these will be the last patches for a while.
> 
> JÃrn
> 

The problem is that the hardware does not support another callback. In
any case while you are right that the function might be called as second
time, in the original code it will sleep while waiting for the lock,
which is allocated per device.

On the second patch - aiui completions do an uninterruptible wait, that
means they are surely not a good choice for this - especially as the
device could be unplugged at any time. (Admittedly my documentation
migght be of date here)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/