Re: [patch 1/3] ps3: Disk Storage Driver

On Thu, 19 Jul 2007 10:57:53 +0200 (CEST) Geert Uytterhoeven [off-list ref] wrote:

	Hi Andrew,

On Wed, 18 Jul 2007, Andrew Morton wrote:

quoted

On Mon, 16 Jul 2007 18:15:40 +0200
Geert Uytterhoeven [off-list ref] wrote:

quoted

From: Geert Uytterhoeven <redacted>

Add a Disk Storage Driver for the PS3:

Your patchset significantly hits powerpc, scsi and block.  So who gets to
merge this?  Jens?  James?  Paul?

Me, I guess ;)

As Paul is on holidays, please take it.

OK.

The PS3 storage driver core support is
already in mainline, but the actual drivers aren't, as Paul was waiting for
acks from the maintainers.

BTW, do you prefer incremental patches for the comments below, or an update of
the full patchset?

Incremental is preferred, but I convert replacement patches into
incremental patches at about 10Hz nowadays.  Whatever's easier.

(Actually, if it's a replacement patch then only I get to see the
incremental patch, and the incremental patch is more reviewer-friendly).

I didn't have much choice, as most of it was static and I don't need the full
libata core anyway.

If I would factor it out, any good suggestion where to put the factored out
code?

Take it up with Jeff, please.  If you're keen.  It isn't a lot of code.

quoted

quoted

+static int ps3disk_remove(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	struct ps3disk_private *priv = ps3disk_priv(dev);
+
+	__clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
+		    &ps3disk_mask);

I see no locking here which makes this __clear_bit and the above __set_bit
non-racy?

Were .probe()/.remove() made concurrent again? I thought that idea was dropped
because it caused too many problems?

I don't _think_ there's any global exclusion on ->probe calls.  For a
particular driver instance it's hard to see how these thigns can run
concurrently, dunno.  I guess two hotunplugs coud happen concurrently.