LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [git pull] ieee1394 updates
@ 2008-11-06 17:31 Stefan Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2008-11-06 17:31 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive one post .27 regression fix, one fix for an old corner-case
bug, and two things that may be questionable for a pull request in the
-rc phase but I'd like to get rid of:

Kay Sievers (2):
      firewire: struct device - replace bus_id with dev_name(), dev_set_name()
      ieee1394: struct device - replace bus_id with dev_name(), dev_set_name()

Stefan Richter (2):
      ieee1394: raw1394: fix possible deadlock in multithreaded clients
      ieee1394: dv1394: fix possible deadlock in multithreaded clients

 drivers/firewire/fw-device.c |   14 ++++++--------
 drivers/firewire/fw-ohci.c   |    2 +-
 drivers/firewire/fw-sbp2.c   |    2 +-
 drivers/ieee1394/dv1394.c    |   10 ++++++++--
 drivers/ieee1394/hosts.c     |    4 ++--
 drivers/ieee1394/nodemgr.c   |   14 +++++---------
 drivers/ieee1394/raw1394.c   |    9 ++++++---
 7 files changed, 29 insertions(+), 26 deletions(-)


commit 8449fc3ae58bf8ee5acbd2280754cde67b5db128
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sun Oct 26 12:02:03 2008 +0100

    ieee1394: dv1394: fix possible deadlock in multithreaded clients
    
    Fix a possible though highly unlikely deadlock:
    
    Thread A:                  Thread B:
     - acquire mmap_sem         - dv1394_ioctl/read/write()
     - dv1394_mmap()            - acquire video->mtx
     - acquire video->mtx       - copy_to/from_user(), possible page fault:
                                  acquire mmap_sem
    
    The simplest fix is to use mutex_trylock() instead of mutex_lock() in
    dv1394_mmap().  This changes the behavior under contention in a way
    which is visible to userspace clients.  However, my guess is that no
    clients exist which use mmap vs. ioctl/read/write on the dv1394
    character device file interface in concurrent threads.
    
    Reported-by: Johannes Weiner <hannes@saeurebad.de>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/dv1394.c b/drivers/ieee1394/dv1394.c
index df70f51..5332997 100644
--- a/drivers/ieee1394/dv1394.c
+++ b/drivers/ieee1394/dv1394.c
@@ -1270,8 +1270,14 @@ static int dv1394_mmap(struct file *file, struct vm_area_struct *vma)
 	struct video_card *video = file_to_video_card(file);
 	int retval = -EINVAL;
 
-	/* serialize mmap */
-	mutex_lock(&video->mtx);
+	/*
+	 * We cannot use the blocking variant mutex_lock here because .mmap
+	 * is called with mmap_sem held, while .ioctl, .read, .write acquire
+	 * video->mtx and subsequently call copy_to/from_user which will
+	 * grab mmap_sem in case of a page fault.
+	 */
+	if (!mutex_trylock(&video->mtx))
+		return -EAGAIN;
 
 	if ( ! video_card_initialized(video) ) {
 		retval = do_dv1394_init_default(video);

commit 638570b54346f140bc09b986d93e76025d35180f
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sun Oct 26 12:03:37 2008 +0100

    ieee1394: raw1394: fix possible deadlock in multithreaded clients
    
    Regression in 2.6.28-rc1:  When I added the new state_mutex which
    prevents corruption of raw1394's internal state when accessed by
    multithreaded client applications, the following possible though
    highly unlikely deadlock slipped in:
    
    Thread A:                  Thread B:
     - acquire mmap_sem         - raw1394_write() or raw1394_ioctl()
     - raw1394_mmap()           - acquire state_mutex
     - acquire state_mutex      - copy_to/from_user(), possible page fault:
                                  acquire mmap_sem
    
    The simplest fix is to use mutex_trylock() instead of mutex_lock() in
    raw1394_mmap().  This changes the behavior under contention in a way
    which is visible to userspace clients.  However, since multithreaded
    access was entirely buggy before state_mutex was added and libraw1394's
    documentation advised application programmers to use a handle only in a
    single thread, this change in behaviour should not be an issue in
    practice at all.
    
    Since we have to use mutex_trylock() in raw1394_mmap() regardless
    whether /dev/raw1394 was opened with O_NONBLOCK or not, we now use
    mutex_trylock() unconditionally everywhere for state_mutex, just to have
    consistent behavior.
    
    Reported-by: Johannes Weiner <hannes@saeurebad.de>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/raw1394.c b/drivers/ieee1394/raw1394.c
index 2cf4ae7..4bdfff0 100644
--- a/drivers/ieee1394/raw1394.c
+++ b/drivers/ieee1394/raw1394.c
@@ -2268,7 +2268,8 @@ static ssize_t raw1394_write(struct file *file, const char __user * buffer,
 		return -EFAULT;
 	}
 
-	mutex_lock(&fi->state_mutex);
+	if (!mutex_trylock(&fi->state_mutex))
+		return -EAGAIN;
 
 	switch (fi->state) {
 	case opened:
@@ -2548,7 +2549,8 @@ static int raw1394_mmap(struct file *file, struct vm_area_struct *vma)
 	struct file_info *fi = file->private_data;
 	int ret;
 
-	mutex_lock(&fi->state_mutex);
+	if (!mutex_trylock(&fi->state_mutex))
+		return -EAGAIN;
 
 	if (fi->iso_state == RAW1394_ISO_INACTIVE)
 		ret = -EINVAL;
@@ -2669,7 +2671,8 @@ static long raw1394_ioctl(struct file *file, unsigned int cmd,
 		break;
 	}
 
-	mutex_lock(&fi->state_mutex);
+	if (!mutex_trylock(&fi->state_mutex))
+		return -EAGAIN;
 
 	switch (fi->iso_state) {
 	case RAW1394_ISO_INACTIVE:

commit 233976e539a93de1320fc7625b24076b1f9e2c9c
Author: Kay Sievers <kay.sievers@vrfy.org>
Date:   Thu Oct 30 01:49:20 2008 +0100

    ieee1394: struct device - replace bus_id with dev_name(), dev_set_name()
    
    Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
    Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/hosts.c b/drivers/ieee1394/hosts.c
index 8dd09d8..237d0c9 100644
--- a/drivers/ieee1394/hosts.c
+++ b/drivers/ieee1394/hosts.c
@@ -155,11 +155,11 @@ struct hpsb_host *hpsb_alloc_host(struct hpsb_host_driver *drv, size_t extra,
 	memcpy(&h->device, &nodemgr_dev_template_host, sizeof(h->device));
 	h->device.parent = dev;
 	set_dev_node(&h->device, dev_to_node(dev));
-	snprintf(h->device.bus_id, BUS_ID_SIZE, "fw-host%d", h->id);
+	dev_set_name(&h->device, "fw-host%d", h->id);
 
 	h->host_dev.parent = &h->device;
 	h->host_dev.class = &hpsb_host_class;
-	snprintf(h->host_dev.bus_id, BUS_ID_SIZE, "fw-host%d", h->id);
+	dev_set_name(&h->host_dev, "fw-host%d", h->id);
 
 	if (device_register(&h->device))
 		goto fail;
diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c
index 2376b72..9e39f73 100644
--- a/drivers/ieee1394/nodemgr.c
+++ b/drivers/ieee1394/nodemgr.c
@@ -826,13 +826,11 @@ static struct node_entry *nodemgr_create_node(octlet_t guid,
 	memcpy(&ne->device, &nodemgr_dev_template_ne,
 	       sizeof(ne->device));
 	ne->device.parent = &host->device;
-	snprintf(ne->device.bus_id, BUS_ID_SIZE, "%016Lx",
-		 (unsigned long long)(ne->guid));
+	dev_set_name(&ne->device, "%016Lx", (unsigned long long)(ne->guid));
 
 	ne->node_dev.parent = &ne->device;
 	ne->node_dev.class = &nodemgr_ne_class;
-	snprintf(ne->node_dev.bus_id, BUS_ID_SIZE, "%016Lx",
-		(unsigned long long)(ne->guid));
+	dev_set_name(&ne->node_dev, "%016Lx", (unsigned long long)(ne->guid));
 
 	if (device_register(&ne->device))
 		goto fail_devreg;
@@ -932,13 +930,11 @@ static void nodemgr_register_device(struct node_entry *ne,
 
 	ud->device.parent = parent;
 
-	snprintf(ud->device.bus_id, BUS_ID_SIZE, "%s-%u",
-		 ne->device.bus_id, ud->id);
+	dev_set_name(&ud->device, "%s-%u", dev_name(&ne->device), ud->id);
 
 	ud->unit_dev.parent = &ud->device;
 	ud->unit_dev.class = &nodemgr_ud_class;
-	snprintf(ud->unit_dev.bus_id, BUS_ID_SIZE, "%s-%u",
-		 ne->device.bus_id, ud->id);
+	dev_set_name(&ud->unit_dev, "%s-%u", dev_name(&ne->device), ud->id);
 
 	if (device_register(&ud->device))
 		goto fail_devreg;
@@ -953,7 +949,7 @@ static void nodemgr_register_device(struct node_entry *ne,
 fail_classdevreg:
 	device_unregister(&ud->device);
 fail_devreg:
-	HPSB_ERR("Failed to create unit %s", ud->device.bus_id);
+	HPSB_ERR("Failed to create unit %s", dev_name(&ud->device));
 }	
 
 

commit a1f64819fe9f136c98d572794a35a7e377c951ef
Author: Kay Sievers <kay.sievers@vrfy.org>
Date:   Thu Oct 30 01:41:56 2008 +0100

    firewire: struct device - replace bus_id with dev_name(), dev_set_name()
    
    Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
    Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c
index 3fccdd4..6b9be42 100644
--- a/drivers/firewire/fw-device.c
+++ b/drivers/firewire/fw-device.c
@@ -587,8 +587,7 @@ static void create_units(struct fw_device *device)
 		unit->device.bus = &fw_bus_type;
 		unit->device.type = &fw_unit_type;
 		unit->device.parent = &device->device;
-		snprintf(unit->device.bus_id, sizeof(unit->device.bus_id),
-			 "%s.%d", device->device.bus_id, i++);
+		dev_set_name(&unit->device, "%s.%d", dev_name(&device->device), i++);
 
 		init_fw_attribute_group(&unit->device,
 					fw_unit_attributes,
@@ -711,8 +710,7 @@ static void fw_device_init(struct work_struct *work)
 	device->device.type = &fw_device_type;
 	device->device.parent = device->card->device;
 	device->device.devt = MKDEV(fw_cdev_major, minor);
-	snprintf(device->device.bus_id, sizeof(device->device.bus_id),
-		 "fw%d", minor);
+	dev_set_name(&device->device, "fw%d", minor);
 
 	init_fw_attribute_group(&device->device,
 				fw_device_attributes,
@@ -741,13 +739,13 @@ static void fw_device_init(struct work_struct *work)
 		if (device->config_rom_retries)
 			fw_notify("created device %s: GUID %08x%08x, S%d00, "
 				  "%d config ROM retries\n",
-				  device->device.bus_id,
+				  dev_name(&device->device),
 				  device->config_rom[3], device->config_rom[4],
 				  1 << device->max_speed,
 				  device->config_rom_retries);
 		else
 			fw_notify("created device %s: GUID %08x%08x, S%d00\n",
-				  device->device.bus_id,
+				  dev_name(&device->device),
 				  device->config_rom[3], device->config_rom[4],
 				  1 << device->max_speed);
 		device->config_rom_retries = 0;
@@ -883,12 +881,12 @@ static void fw_device_refresh(struct work_struct *work)
 		    FW_DEVICE_RUNNING) == FW_DEVICE_SHUTDOWN)
 		goto gone;
 
-	fw_notify("refreshed device %s\n", device->device.bus_id);
+	fw_notify("refreshed device %s\n", dev_name(&device->device));
 	device->config_rom_retries = 0;
 	goto out;
 
  give_up:
-	fw_notify("giving up on refresh of device %s\n", device->device.bus_id);
+	fw_notify("giving up on refresh of device %s\n", dev_name(&device->device));
  gone:
 	atomic_set(&device->state, FW_DEVICE_SHUTDOWN);
 	fw_device_shutdown(work);
diff --git a/drivers/firewire/fw-ohci.c b/drivers/firewire/fw-ohci.c
index 8e16bfb..46610b0 100644
--- a/drivers/firewire/fw-ohci.c
+++ b/drivers/firewire/fw-ohci.c
@@ -2468,7 +2468,7 @@ pci_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 		goto fail_self_id;
 
 	fw_notify("Added fw-ohci device %s, OHCI version %x.%x\n",
-		  dev->dev.bus_id, version >> 16, version & 0xff);
+		  dev_name(&dev->dev), version >> 16, version & 0xff);
 	return 0;
 
  fail_self_id:
diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index d334cac..97df6da 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -1135,7 +1135,7 @@ static int sbp2_probe(struct device *dev)
 	tgt->unit = unit;
 	kref_init(&tgt->kref);
 	INIT_LIST_HEAD(&tgt->lu_list);
-	tgt->bus_id = unit->device.bus_id;
+	tgt->bus_id = dev_name(&unit->device);
 	tgt->guid = (u64)device->config_rom[3] << 32 | device->config_rom[4];
 
 	if (fw_device_enable_phys_dma(device) < 0)

Thanks,
-- 
Stefan Richter
-=====-==--- =-== --==-
http://arcgraph.de/sr/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [git pull] ieee1394 updates
@ 2008-12-01 17:03 Stefan Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2008-12-01 17:03 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

the following updates:

Stefan Richter (4):
      ieee1394: sbp2: another iPod mini quirk entry
      firewire: fw-sbp2: another iPod mini quirk entry
      ieee1394: fix list corruption (reported at module removal)
      ieee1394: sbp2: fix race condition in state change

 drivers/firewire/fw-sbp2.c   |    5 +++++
 drivers/ieee1394/highlevel.c |   25 ++++++++++++-------------
 drivers/ieee1394/hosts.h     |    4 ++++
 drivers/ieee1394/sbp2.c      |   14 ++++++++++----
 4 files changed, 31 insertions(+), 17 deletions(-)


commit 2642b11295ebcc94843045933061bfbb263fce7f
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Nov 29 14:55:47 2008 +0100

    ieee1394: sbp2: fix race condition in state change
    
    An intermediate transition from _RUNNING to _IN_SHUTDOWN could have been
    missed by the former code.
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index 3f5dbcb..a373c18 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -895,12 +895,13 @@ static void sbp2_host_reset(struct hpsb_host *host)
 		return;
 
 	read_lock_irqsave(&sbp2_hi_logical_units_lock, flags);
+
 	list_for_each_entry(lu, &hi->logical_units, lu_list)
-		if (likely(atomic_read(&lu->state) !=
-			   SBP2LU_STATE_IN_SHUTDOWN)) {
-			atomic_set(&lu->state, SBP2LU_STATE_IN_RESET);
+		if (atomic_cmpxchg(&lu->state,
+				   SBP2LU_STATE_RUNNING, SBP2LU_STATE_IN_RESET)
+		    == SBP2LU_STATE_RUNNING)
 			scsi_block_requests(lu->shost);
-		}
+
 	read_unlock_irqrestore(&sbp2_hi_logical_units_lock, flags);
 }
 

commit e47c1feb17e61ef4e2f245c0af0c5a8e2a7798b2
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Wed Nov 26 01:34:25 2008 +0100

    ieee1394: fix list corruption (reported at module removal)
    
    If there is more than one FireWire controller present, dummy_zero_addr
    and dummy_max_addr were added multiple times to different lists, thus
    corrupting the lists.  Fix this by allocating them dynamically per host
    instead of just once globally.
    
    (Perhaps a better address space allocation algorithm could rid us of the
    two dummy address spaces.)
    
    Fixes http://bugzilla.kernel.org/show_bug.cgi?id=10129 .
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/highlevel.c b/drivers/ieee1394/highlevel.c
index 918ffc4..272543a 100644
--- a/drivers/ieee1394/highlevel.c
+++ b/drivers/ieee1394/highlevel.c
@@ -46,10 +46,6 @@ static DEFINE_RWLOCK(hl_irqs_lock);
 
 static DEFINE_RWLOCK(addr_space_lock);
 
-/* addr_space list will have zero and max already included as bounds */
-static struct hpsb_address_ops dummy_ops = { NULL, NULL, NULL, NULL };
-static struct hpsb_address_serve dummy_zero_addr, dummy_max_addr;
-
 
 static struct hl_host_info *hl_get_hostinfo(struct hpsb_highlevel *hl,
 					    struct hpsb_host *host)
@@ -481,20 +477,23 @@ int hpsb_unregister_addrspace(struct hpsb_highlevel *hl, struct hpsb_host *host,
 	return retval;
 }
 
+static struct hpsb_address_ops dummy_ops;
+
+/* dummy address spaces as lower and upper bounds of the host's a.s. list */
 static void init_hpsb_highlevel(struct hpsb_host *host)
 {
-	INIT_LIST_HEAD(&dummy_zero_addr.host_list);
-	INIT_LIST_HEAD(&dummy_zero_addr.hl_list);
-	INIT_LIST_HEAD(&dummy_max_addr.host_list);
-	INIT_LIST_HEAD(&dummy_max_addr.hl_list);
+	INIT_LIST_HEAD(&host->dummy_zero_addr.host_list);
+	INIT_LIST_HEAD(&host->dummy_zero_addr.hl_list);
+	INIT_LIST_HEAD(&host->dummy_max_addr.host_list);
+	INIT_LIST_HEAD(&host->dummy_max_addr.hl_list);
 
-	dummy_zero_addr.op = dummy_max_addr.op = &dummy_ops;
+	host->dummy_zero_addr.op = host->dummy_max_addr.op = &dummy_ops;
 
-	dummy_zero_addr.start = dummy_zero_addr.end = 0;
-	dummy_max_addr.start = dummy_max_addr.end = ((u64) 1) << 48;
+	host->dummy_zero_addr.start = host->dummy_zero_addr.end = 0;
+	host->dummy_max_addr.start = host->dummy_max_addr.end = ((u64) 1) << 48;
 
-	list_add_tail(&dummy_zero_addr.host_list, &host->addr_space);
-	list_add_tail(&dummy_max_addr.host_list, &host->addr_space);
+	list_add_tail(&host->dummy_zero_addr.host_list, &host->addr_space);
+	list_add_tail(&host->dummy_max_addr.host_list, &host->addr_space);
 }
 
 void highlevel_add_host(struct hpsb_host *host)
diff --git a/drivers/ieee1394/hosts.h b/drivers/ieee1394/hosts.h
index e4e8aeb..dd22995 100644
--- a/drivers/ieee1394/hosts.h
+++ b/drivers/ieee1394/hosts.h
@@ -13,6 +13,7 @@ struct module;
 
 #include "ieee1394_types.h"
 #include "csr.h"
+#include "highlevel.h"
 
 struct hpsb_packet;
 struct hpsb_iso;
@@ -72,6 +73,9 @@ struct hpsb_host {
 	struct { DECLARE_BITMAP(map, 64); } tl_pool[ALL_NODES];
 
 	struct csr_control csr;
+
+	struct hpsb_address_serve dummy_zero_addr;
+	struct hpsb_address_serve dummy_max_addr;
 };
 
 enum devctl_cmd {

commit 031bb27c4bf77c2f60b3f3dea8cce63ef0d1fba9
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Nov 22 12:38:58 2008 +0100

    firewire: fw-sbp2: another iPod mini quirk entry
    
    Add another model ID of a broken firmware to prevent early I/O errors
    by acesses at the end of the disk.  Reported at linux1394-user,
    http://marc.info/?t=122670842900002
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index 97df6da..e54403e 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -372,6 +372,11 @@ static const struct {
 	},
 	/* iPod mini */ {
 		.firmware_revision	= 0x0a2700,
+		.model			= 0x000022,
+		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
+	},
+	/* iPod mini */ {
+		.firmware_revision	= 0x0a2700,
 		.model			= 0x000023,
 		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
 	},

commit 9e0de91011ef6fe6eb3bb63f7ea15f586955660a
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Nov 22 12:38:24 2008 +0100

    ieee1394: sbp2: another iPod mini quirk entry
    
    Add another model ID of a broken firmware to prevent early I/O errors
    by acesses at the end of the disk.  Reported at linux1394-user,
    http://marc.info/?t=122670842900002
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index c52f6e6..3f5dbcb 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -402,6 +402,11 @@ static const struct {
 	},
 	/* iPod mini */ {
 		.firmware_revision	= 0x0a2700,
+		.model_id		= 0x000022,
+		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
+	},
+	/* iPod mini */ {
+		.firmware_revision	= 0x0a2700,
 		.model_id		= 0x000023,
 		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
 	},


Thanks,
-- 
Stefan Richter
-=====-==--- ==-- ----=
http://arcgraph.de/sr/



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [git pull] ieee1394 updates
@ 2008-08-19 17:10 Stefan Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2008-08-19 17:10 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive the following ieee1394 and firewire subsystem updates.  The
ieee1394 part fixes and enhances SBP-2 reconnection when further devices
are plugged in to a bus or become ready while other devices are in use.

The -stable team will receive a backport of the regression fix.


Stefan Richter (4):
      ieee1394: regression in 2.6.25: updates should happen before probes
      ieee1394: don't drop nodes during bus reset series
      ieee1394: sbp2: let nodemgr retry node updates during bus reset series
      firewire: Kconfig help update

 drivers/firewire/Kconfig   |    4 +-
 drivers/ieee1394/nodemgr.c |   63 +++++++++++++++++++-----------------
 drivers/ieee1394/nodemgr.h |    2 +-
 drivers/ieee1394/sbp2.c    |   25 ++++++++++----
 4 files changed, 54 insertions(+), 40 deletions(-)


commit 30b0aa7c9a5e769a874a456cd56396eebf164b91
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Aug 16 21:52:28 2008 +0200

    firewire: Kconfig help update
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig
index fa6d6ab..4509024 100644
--- a/drivers/firewire/Kconfig
+++ b/drivers/firewire/Kconfig
@@ -12,8 +12,8 @@ config FIREWIRE
 	  This is the "Juju" FireWire stack, a new alternative implementation
 	  designed for robustness and simplicity.  You can build either this
 	  stack, or the old stack (the ieee1394 driver, ohci1394 etc.) or both.
-	  Please read http://wiki.linux1394.org/JujuMigration before you
-	  enable the new stack.
+	  Please read http://ieee1394.wiki.kernel.org/index.php/Juju_Migration
+	  before you enable the new stack.
 
 	  To compile this driver as a module, say M here: the module will be
 	  called firewire-core.

commit a3384067fb0df9c58e112ac6a5ec9beb7d169482
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Aug 16 13:39:26 2008 +0200

    ieee1394: sbp2: let nodemgr retry node updates during bus reset series
    
    sbp2 was too quick to report .update() to the ieee1394 core as failed.
    (Logged as "Failed to reconnect to sbp2 device!".)  The core would then
    unbind sbp2 from the device.
    
    This is not justified if the .update() failed because another bus reset
    happened.  We check this and tell the ieee1394 that .update() succeeded,
    and the core will call sbp2's .update() for the new bus reset as well.
    
    This improves reconnection/re-login especially on buses with several
    disks as they may issue bus resets in close succession when they come
    online.
    
    Tested by Damien Benoist.
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index 9cbf315..1d6ad34 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -731,15 +731,26 @@ static int sbp2_update(struct unit_directory *ud)
 {
 	struct sbp2_lu *lu = ud->device.driver_data;
 
-	if (sbp2_reconnect_device(lu)) {
-		/* Reconnect has failed. Perhaps we didn't reconnect fast
-		 * enough. Try a regular login, but first log out just in
-		 * case of any weirdness. */
+	if (sbp2_reconnect_device(lu) != 0) {
+		/*
+		 * Reconnect failed.  If another bus reset happened,
+		 * let nodemgr proceed and call sbp2_update again later
+		 * (or sbp2_remove if this node went away).
+		 */
+		if (!hpsb_node_entry_valid(lu->ne))
+			return 0;
+		/*
+		 * Or the target rejected the reconnect because we weren't
+		 * fast enough.  Try a regular login, but first log out
+		 * just in case of any weirdness.
+		 */
 		sbp2_logout_device(lu);
 
-		if (sbp2_login_device(lu)) {
-			/* Login failed too, just fail, and the backend
-			 * will call our sbp2_remove for us */
+		if (sbp2_login_device(lu) != 0) {
+			if (!hpsb_node_entry_valid(lu->ne))
+				return 0;
+
+			/* Maybe another initiator won the login. */
 			SBP2_ERR("Failed to reconnect to sbp2 device!");
 			return -EBUSY;
 		}

commit c921a9745705ed62a949192ef9128c60d6c63874
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Aug 16 13:38:11 2008 +0200

    ieee1394: don't drop nodes during bus reset series
    
    nodemgr_node_probe checked for generation increments too late and
    therefore prematurely reported nodes as "suspended".
    
    Fixes http://bugzilla.kernel.org/show_bug.cgi?id=11349.  Reported and
    tested by Damien Benoist.
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c
index 2ebd09a..16240a7 100644
--- a/drivers/ieee1394/nodemgr.c
+++ b/drivers/ieee1394/nodemgr.c
@@ -1574,6 +1574,9 @@ static int node_probe(struct device *dev, void *data)
 	struct probe_param *p = data;
 	struct node_entry *ne;
 
+	if (p->generation != get_hpsb_generation(p->hi->host))
+		return -EAGAIN;
+
 	ne = container_of(dev, struct node_entry, node_dev);
 	if (ne->needs_probe == p->probe_now)
 		nodemgr_probe_ne(p->hi, ne, p->generation);
@@ -1582,42 +1585,41 @@ static int node_probe(struct device *dev, void *data)
 
 static void nodemgr_node_probe(struct host_info *hi, int generation)
 {
-	struct hpsb_host *host = hi->host;
 	struct probe_param p;
 
 	p.hi = hi;
 	p.generation = generation;
-	/* Do some processing of the nodes we've probed. This pulls them
+	/*
+	 * Do some processing of the nodes we've probed. This pulls them
 	 * into the sysfs layer if needed, and can result in processing of
 	 * unit-directories, or just updating the node and it's
 	 * unit-directories.
 	 *
 	 * Run updates before probes. Usually, updates are time-critical
-	 * while probes are time-consuming. (Well, those probes need some
-	 * improvement...) */
-
+	 * while probes are time-consuming.
+	 *
+	 * Meanwhile, another bus reset may have happened. In this case we
+	 * skip everything here and let the next bus scan handle it.
+	 * Otherwise we may prematurely remove nodes which are still there.
+	 */
 	p.probe_now = false;
-	class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe);
-	p.probe_now = true;
-	class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe);
+	if (class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe) != 0)
+		return;
 
-	/* If we had a bus reset while we were scanning the bus, it is
-	 * possible that we did not probe all nodes.  In that case, we
-	 * skip the clean up for now, since we could remove nodes that
-	 * were still on the bus.  Another bus scan is pending which will
-	 * do the clean up eventually.
-	 *
+	p.probe_now = true;
+	if (class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe) != 0)
+		return;
+	/*
 	 * Now let's tell the bus to rescan our devices. This may seem
 	 * like overhead, but the driver-model core will only scan a
 	 * device for a driver when either the device is added, or when a
 	 * new driver is added. A bus reset is a good reason to rescan
 	 * devices that were there before.  For example, an sbp2 device
 	 * may become available for login, if the host that held it was
-	 * just removed.  */
-
-	if (generation == get_hpsb_generation(host))
-		if (bus_rescan_devices(&ieee1394_bus_type))
-			HPSB_DEBUG("bus_rescan_devices had an error");
+	 * just removed.
+	 */
+	if (bus_rescan_devices(&ieee1394_bus_type) != 0)
+		HPSB_DEBUG("bus_rescan_devices had an error");
 }
 
 static int nodemgr_send_resume_packet(struct hpsb_host *host)

commit 6848408abf1bc18d9a4d5fed3fcca812745ece05
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sat Aug 16 13:36:47 2008 +0200

    ieee1394: regression in 2.6.25: updates should happen before probes
    
    Regression since commit 73cf60232ef16e1f8a64defa97214a1722db1e6c,
    "ieee1394: use class iteration api":  The two loops for (1.) driver
    updates and (2.) driver probes were replaced by a single loop with
    bogus needs_probe checks.  Hence updates and probes were now intermixed,
    and especially sbp2 updates (reconnects) held up longer than necessary.
    
    While we fix it, change the needs_probe flag to bool type for clarity.
    
    Tested by Damien Benoist.
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/nodemgr.c b/drivers/ieee1394/nodemgr.c
index 994a21e..2ebd09a 100644
--- a/drivers/ieee1394/nodemgr.c
+++ b/drivers/ieee1394/nodemgr.c
@@ -844,7 +844,7 @@ static struct node_entry *nodemgr_create_node(octlet_t guid, struct csr1212_csr
 	ne->host = host;
 	ne->nodeid = nodeid;
 	ne->generation = generation;
-	ne->needs_probe = 1;
+	ne->needs_probe = true;
 
 	ne->guid = guid;
 	ne->guid_vendor_id = (guid >> 40) & 0xffffff;
@@ -1144,7 +1144,7 @@ static void nodemgr_process_root_directory(struct host_info *hi, struct node_ent
 	struct csr1212_keyval *kv, *vendor_name_kv = NULL;
 	u8 last_key_id = 0;
 
-	ne->needs_probe = 0;
+	ne->needs_probe = false;
 
 	csr1212_for_each_dir_entry(ne->csr, kv, ne->csr->root_kv, dentry) {
 		switch (kv->key.id) {
@@ -1295,7 +1295,7 @@ static void nodemgr_update_node(struct node_entry *ne, struct csr1212_csr *csr,
 		nodemgr_update_bus_options(ne);
 
 		/* Mark the node as new, so it gets re-probed */
-		ne->needs_probe = 1;
+		ne->needs_probe = true;
 	} else {
 		/* old cache is valid, so update its generation */
 		struct nodemgr_csr_info *ci = ne->csr->private;
@@ -1566,28 +1566,27 @@ static void nodemgr_probe_ne(struct host_info *hi, struct node_entry *ne, int ge
 struct probe_param {
 	struct host_info *hi;
 	int generation;
+	bool probe_now;
 };
 
-static int __nodemgr_node_probe(struct device *dev, void *data)
+static int node_probe(struct device *dev, void *data)
 {
-	struct probe_param *param = (struct probe_param *)data;
+	struct probe_param *p = data;
 	struct node_entry *ne;
 
 	ne = container_of(dev, struct node_entry, node_dev);
-	if (!ne->needs_probe)
-		nodemgr_probe_ne(param->hi, ne, param->generation);
-	if (ne->needs_probe)
-		nodemgr_probe_ne(param->hi, ne, param->generation);
+	if (ne->needs_probe == p->probe_now)
+		nodemgr_probe_ne(p->hi, ne, p->generation);
 	return 0;
 }
 
 static void nodemgr_node_probe(struct host_info *hi, int generation)
 {
 	struct hpsb_host *host = hi->host;
-	struct probe_param param;
+	struct probe_param p;
 
-	param.hi = hi;
-	param.generation = generation;
+	p.hi = hi;
+	p.generation = generation;
 	/* Do some processing of the nodes we've probed. This pulls them
 	 * into the sysfs layer if needed, and can result in processing of
 	 * unit-directories, or just updating the node and it's
@@ -1597,8 +1596,10 @@ static void nodemgr_node_probe(struct host_info *hi, int generation)
 	 * while probes are time-consuming. (Well, those probes need some
 	 * improvement...) */
 
-	class_for_each_device(&nodemgr_ne_class, NULL, &param,
-			      __nodemgr_node_probe);
+	p.probe_now = false;
+	class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe);
+	p.probe_now = true;
+	class_for_each_device(&nodemgr_ne_class, NULL, &p, node_probe);
 
 	/* If we had a bus reset while we were scanning the bus, it is
 	 * possible that we did not probe all nodes.  In that case, we
diff --git a/drivers/ieee1394/nodemgr.h b/drivers/ieee1394/nodemgr.h
index 919e92e..6eb2646 100644
--- a/drivers/ieee1394/nodemgr.h
+++ b/drivers/ieee1394/nodemgr.h
@@ -97,7 +97,7 @@ struct node_entry {
 	struct hpsb_host *host;		/* Host this node is attached to */
 	nodeid_t nodeid;		/* NodeID */
 	struct bus_options busopt;	/* Bus Options */
-	int needs_probe;
+	bool needs_probe;
 	unsigned int generation;	/* Synced with hpsb generation */
 
 	/* The following is read from the config rom */


Thanks,
-- 
Stefan Richter
-=====-==--- =--- =--==
http://arcgraph.de/sr/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [GIT PULL] ieee1394 updates
@ 2008-05-20 16:46 Stefan Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2008-05-20 16:46 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive the following IEEE 1394/ FireWire subsystem updates.

Jay Fenlason (1):
      firewire: prevent userspace from accessing shut down devices

Stefan Richter (1):
      ieee1394: sbp2: use correct size of command descriptor block

 drivers/firewire/fw-cdev.c |   14 ++++++++++++++
 drivers/ieee1394/sbp2.c    |   20 ++++++++------------
 2 files changed, 22 insertions(+), 12 deletions(-)


commit 551f4cb9de716ffcdaf968c99a450c22ff12e8c3
Author: Jay Fenlason <fenlason@redhat.com>
Date:   Fri May 16 11:15:23 2008 -0400

    firewire: prevent userspace from accessing shut down devices
    
    If userspace ignores the POLLERR bit from poll(), and only attempts to
    read() the device when POLLIN is set, it can still make ioctl() calls on
    a device that has been removed from the system.  The node_id and
    generation returned by GET_INFO will be outdated, but INITIATE_BUS_RESET
    would still cause a bus reset, and GET_CYCLE_TIMER will return data.
    And if you guess the correct generation to use, you can send requests to
    a different device on the bus, and get responses back.
    
    This patch prevents open, ioctl, compat_ioctl, and mmap against shutdown
    devices.
    
    Signed-off-by: Jay Fenlason <fenlason@redhat.com>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c
index 4a54192..dda1401 100644
--- a/drivers/firewire/fw-cdev.c
+++ b/drivers/firewire/fw-cdev.c
@@ -113,6 +113,11 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
 	if (device == NULL)
 		return -ENODEV;
 
+	if (fw_device_is_shutdown(device)) {
+		fw_device_put(device);
+		return -ENODEV;
+	}
+
 	client = kzalloc(sizeof(*client), GFP_KERNEL);
 	if (client == NULL) {
 		fw_device_put(device);
@@ -901,6 +906,9 @@ fw_device_op_ioctl(struct file *file,
 {
 	struct client *client = file->private_data;
 
+	if (fw_device_is_shutdown(client->device))
+		return -ENODEV;
+
 	return dispatch_ioctl(client, cmd, (void __user *) arg);
 }
 
@@ -911,6 +919,9 @@ fw_device_op_compat_ioctl(struct file *file,
 {
 	struct client *client = file->private_data;
 
+	if (fw_device_is_shutdown(client->device))
+		return -ENODEV;
+
 	return dispatch_ioctl(client, cmd, compat_ptr(arg));
 }
 #endif
@@ -922,6 +933,9 @@ static int fw_device_op_mmap(struct file *file, struct vm_area_struct *vma)
 	unsigned long size;
 	int page_count, retval;
 
+	if (fw_device_is_shutdown(client->device))
+		return -ENODEV;
+
 	/* FIXME: We could support multiple buffers, but we don't. */
 	if (client->buffer.pages != NULL)
 		return -EBUSY;


commit 93c596f7d611b379302bbdd26f31acdf72f4859a
Author: Stefan Richter <stefanr@s5r6.in-berlin.de>
Date:   Sun May 4 16:54:14 2008 +0200

    ieee1394: sbp2: use correct size of command descriptor block
    
    Boaz Harrosh wrote:
    > cmd->cmd_len is now guarantied to be set properly at all cases.
    > And some commands you want to support will not be set correctly
    > by COMMAND_SIZE().
    
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index 16b9d0a..a5ceff2 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -1539,15 +1539,13 @@ static void sbp2_prep_command_orb_sg(struct sbp2_command_orb *orb,
 
 static void sbp2_create_command_orb(struct sbp2_lu *lu,
 				    struct sbp2_command_info *cmd,
-				    unchar *scsi_cmd,
-				    unsigned int scsi_use_sg,
-				    unsigned int scsi_request_bufflen,
-				    struct scatterlist *sg,
-				    enum dma_data_direction dma_dir)
+				    struct scsi_cmnd *SCpnt)
 {
 	struct sbp2_fwhost_info *hi = lu->hi;
 	struct sbp2_command_orb *orb = &cmd->command_orb;
 	u32 orb_direction;
+	unsigned int scsi_request_bufflen = scsi_bufflen(SCpnt);
+	enum dma_data_direction dma_dir = SCpnt->sc_data_direction;
 
 	/*
 	 * Set-up our command ORB.
@@ -1580,13 +1578,14 @@ static void sbp2_create_command_orb(struct sbp2_lu *lu,
 		orb->data_descriptor_lo = 0x0;
 		orb->misc |= ORB_SET_DIRECTION(1);
 	} else
-		sbp2_prep_command_orb_sg(orb, hi, cmd, scsi_use_sg, sg,
+		sbp2_prep_command_orb_sg(orb, hi, cmd, scsi_sg_count(SCpnt),
+					 scsi_sglist(SCpnt),
 					 orb_direction, dma_dir);
 
 	sbp2util_cpu_to_be32_buffer(orb, sizeof(*orb));
 
-	memset(orb->cdb, 0, 12);
-	memcpy(orb->cdb, scsi_cmd, COMMAND_SIZE(*scsi_cmd));
+	memset(orb->cdb, 0, sizeof(orb->cdb));
+	memcpy(orb->cdb, SCpnt->cmnd, SCpnt->cmd_len);
 }
 
 static void sbp2_link_orb_command(struct sbp2_lu *lu,
@@ -1669,16 +1668,13 @@ static void sbp2_link_orb_command(struct sbp2_lu *lu,
 static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,
 			     void (*done)(struct scsi_cmnd *))
 {
-	unchar *scsi_cmd = (unchar *)SCpnt->cmnd;
 	struct sbp2_command_info *cmd;
 
 	cmd = sbp2util_allocate_command_orb(lu, SCpnt, done);
 	if (!cmd)
 		return -EIO;
 
-	sbp2_create_command_orb(lu, cmd, scsi_cmd, scsi_sg_count(SCpnt),
-				scsi_bufflen(SCpnt), scsi_sglist(SCpnt),
-				SCpnt->sc_data_direction);
+	sbp2_create_command_orb(lu, cmd, SCpnt);
 	sbp2_link_orb_command(lu, cmd);
 
 	return 0;


Thanks,
-- 
Stefan Richter
-=====-==--- -=-= =-=--
http://arcgraph.de/sr/


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [GIT PULL] ieee1394 updates
@ 2008-04-25 16:26 Stefan Richter
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Richter @ 2008-04-25 16:26 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton; +Cc: linux-kernel, linux1394-devel

Linus, please pull from the for-linus branch at

    git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git for-linus

to receive the following IEEE 1394 subsystem updates.

Pieter Palmers (1):
      ieee1394: rawiso: requeue packet for transmission after skipped cycle

Tony Breeds (1):
      ieee1394: silence defined but not used warning in non-modular builds

 drivers/ieee1394/dv1394.c    |    2 ++
 drivers/ieee1394/iso.h       |    2 ++
 drivers/ieee1394/ohci1394.c  |   34 ++++++++++++++++++++++++++++++++++
 drivers/ieee1394/raw1394.c   |    9 ++++++++-
 drivers/ieee1394/video1394.c |    2 ++
 5 files changed, 48 insertions(+), 1 deletions(-)

Thanks,
-- 
Stefan Richter
-=====-==--- -=-- ==--=
http://arcgraph.de/sr/


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-12-01 17:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-06 17:31 [git pull] ieee1394 updates Stefan Richter
  -- strict thread matches above, loose matches on Subject: below --
2008-12-01 17:03 Stefan Richter
2008-08-19 17:10 Stefan Richter
2008-05-20 16:46 [GIT PULL] " Stefan Richter
2008-04-25 16:26 Stefan Richter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).