Commit graph

378 commits

Author SHA1 Message Date
Stefan Richter
5d9cb7d276 firewire: cdev: add ioctls for iso resource management, amendment
Some fixes:
  - Remove stale documentation.
  - Fix a != vs. == thinko that got in the way of channel management.
  - Try bandwidth deallocation even if channel deallocation failed.

A simplification:
  - fw_cdev_allocate_iso_resource.channels is now ordered like
    libdc1394's dc1394_iso_allocate_channel() channels_allowed
    argument.

By the way, I looked closer at cards from NEC, TI, and VIA, and noticed
that they all don't implement IEEE 1394a behaviour which is meant to
deviate from IEEE 1212's notion of lock compare-swap.  This means that
we have to do two lock transactions instead of one in many cases where
one transaction would already succeed on a fully 1394a compliant IRM.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:46 +01:00
Stefan Richter
36a755cfc3 firewire: cdev: shut down iso context before freeing the buffer
DMA must be halted before we DMA-unmap and free the DMA buffer.  Since
we cannot rely on the client to stop the context before it closes the
fd, we have to reorder fw_iso_buffer_destroy vs. fw_iso_context_destroy.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:46 +01:00
Stefan Richter
3ba949868a firewire: cdev: replace some spin_lock_irqsave by spin_lock_irq
All of these functions are entered with IRQs enabled.
Hence the unconditional spin_unlock_irq can be used.

Function:                  Caller context:
    dequeue_event()            client process, via read(2)
    fill_bus_reset_event()     fw-device.c update worqueue job
    release_client_resource()  client process, via ioctl(2)
    fw_device_op_release()     client process, via close(2)

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:45 +01:00
Stefan Richter
5d3fd692a7 firewire: cdev: extend transaction payload size check
Make the size check of ioctl_send_request and
ioctl_send_broadcast_request speed dependent.  Also change the error
return code from -EINVAL to -EIO to distinguish this from other errors
concerning the ioctl parameters.

Another payload size limit for which we don't check here though is the
remote node's Bus_Info_Block.max_rec.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:45 +01:00
Stefan Richter
1566f3dc3e firewire: cdev: restrict broadcast write requests to Units Space
We don't want random users write to Memory Space (e.g. PCs with physical
DMA filters down) or to core CSRs like Reset_Start.

This does not protect SBP-2 target CSRs.  But properly behaving SBP-2
targets ignore broadcast write requests to these registers, and the
maximum damage which can happen with laxer targets is DOS.  But there
are ways to create DOS situations anyway if there are devices with weak
device file permissions (like audio/video devices) present at the same
bus as an SBP-2 target.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:45 +01:00
Jay Fenlason, Stefan Richter
acfe833357 firewire: cdev: add ioctl for broadcast write requests
Write transactions to the broadcast node ID are a convenient way to
trigger functions of multiple nodes at once.  IIDC is a protocol which
can make use of this if multiple cameras with same command_regs_base are
connected at the same bus.

Based on
    Date: Wed, 10 Sep 2008 11:32:16 -0400
    From: Jay Fenlason <fenlason@redhat.com>
    Subject: [patch] SEND_BROADCAST_REQUEST
Changes:  ioctl_send_request() and ioctl_send_broadcast_request() now
share code.  Broadcast speed corrected to S100.  Check for proper tcode.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:44 +01:00
Stefan Richter
33580a3ef5 firewire: cdev: add ioctl to query maximum transmission speed
While the speed of asynchronous transactions is automatically chosen by
the kernel, the speed of isochronous streams has to be chosen by the
initiating client.

In case of 1394a bus topologies, the maximum possible speed could be
figured out with some effort by evaluation of the remote node's link
speed field in the config ROM, the local node's link speed field, and
the PHY speeds and topologic information in the local node's or IRM's
topology map CSR.  However, this does not work in case of 1394b buses.

Hence add an ioctl to export the maximum speed which the kernel already
determined.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:44 +01:00
Stefan Richter
1ec3c0269d firewire: cdev: add ioctls for manual iso resource management
This adds ioctls for allocation and deallocation of a channel or/and
bandwidth without auto-reallocation and without auto-deallocation.

The benefit of these ioctls is that libraw1394-style isochronous
resource management can be implemented without write access to the IRM's
character device file.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:44 +01:00
Jay Fenlason, Stefan Richter
b1bda4cdc2 firewire: cdev: add ioctls for isochronous resource management
Based on
    Date: Tue, 18 Nov 2008 11:41:27 -0500
    From: Jay Fenlason <fenlason@redhat.com>
    Subject: [Patch V4] Add ISO resource management support
with several changes to the ABI and implementation.  Only the part of
the ABI which enables auto-reallocation and auto-deallocation is
included here.

This implements ioctls for kernel-assisted allocation of isochronous
channels and isochronous bandwidth.  The benefits are:
  - The client does not have to have write access to the /dev/fw* device
    corresponding to the IRM.
  - The client does not have to perform reallocation after bus resets.
  - Channel and bandwidth are deallocated by the kernel if the file is
    closed before the client deallocated the resources.  Thus resources
    are released even if the client crashes.

It is anticipated that future in-kernel code (firewire-core IRM code;
the firewire port of firedtv), will use the fw-iso.c portions of this
code too.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Tested-by: David Moore <dcm@acm.org>
2009-03-24 20:56:43 +01:00
Stefan Richter
b769bd1765 firewire: core: topology header fix
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:43 +01:00
Stefan Richter
be5bbd6756 firewire: cdev: sort includes
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:43 +01:00
Stefan Richter
97c18b7fd6 firewire: cdev: unify names of struct types and of their instances
to indicate that they are specializations of struct event or of struct
client_resource, respectively.

struct response was both an event and a client_resource; it is now split
into struct outbound_transaction_resource and ~_event in order to
document more explicitly which types of client resources exist.

struct request and struct_request_event are renamed to struct
inbound_transaction_resource and ~_event because requests and responses
occur in outbound and in inbound transactions.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:43 +01:00
Stefan Richter
fb4430367b firewire: cdev: reference-count client instances
The lifetime of struct client instances must be longer than the lifetime
of any client resource.

This fixes a possible race between fw_device_op_release and transaction
completions.  It also prepares for new ioctls for isochronous resource
management which will involve delayed processing of client resources.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Reviewed-by: David Moore <dcm@acm.org>
2009-03-24 20:56:42 +01:00
Stefan Richter
4817ed2402 firewire: prevent creation of multiple IR DMA contexts for the same channel
OHCI-1394 1.1 clause 10.4.3 says:  "If more than one IR DMA context
specifies receives for packets from the same isochronous channel, the
context destination for that channel's packets is undefined."

Any userspace client and in the future also kernelspace clients can
allocate IR DMA contexts for any channel.  We don't want them to
interfere with each other, hence it is preferable to return -EBUSY if
allocation of a second context for a channel is attempted.

Notes:
  - This limitation is OHCI-1394 specific, therefore its proper place of
    implementation is down in the low-level driver.

  - Since the <linux/firewire-cdev.h> ABI simply maps one userspace iso
    client context to one hardware iso context, this OHCI-1394
    limitation alas requires userspace to implement its own multiplexing
    of iso reception from the same channel and card to multiple clients
    when needed.

  - The limitation is independent of channel allocation at the IRM; the
    latter is really only important for the initiation of iso 
    transmission but not of iso reception.

  - We don't need to do the same for IT DMA because OHCI-1394 does not
    have any ties between IT contexts and channels.  Only the voluntary
    channel allocation protocol via the IRM, globally to the FireWire
    bus, can ensure proper isochronous transmit behaviour anyway.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:41 +01:00
Stefan Richter
a459b8ab9c firewire: cdev: use list_first_entry
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:41 +01:00
Stefan Richter
da62df141e firewire: core: remove unused definitions
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:40 +01:00
Stefan Richter
53dca51175 firewire: remove line breaks before function names
type
    function_name(parameters);

is nice to look at but was not used consistently.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:40 +01:00
Stefan Richter
2dbd7d7e23 firewire: standardize a variable name
"ret" is the new "retval".

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:39 +01:00
Stefan Richter
c490a6dec6 firewire: core: remove obsolete assertions
This code never changes.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:39 +01:00
Stefan Richter
44be21b63e firewire: core: remove outdated comment
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:39 +01:00
Stefan Richter
3e0b5f0d7c firewire: cdev: address handler input validation
Like before my commit 1415d9189e,
fw_core_add_address_handler() does not align the address region now.
Instead the caller is required to pass valid parameters.

Since one of the callers of fw_core_add_address_handler() is the cdev
userspace interface, we now check for valid input.  If the client is
buggy, we give it a hint with -EINVAL.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:38 +01:00
Jay Fenlason
45ee3199eb firewire: cdev: use an idr rather than a linked list for resources
The current code uses a linked list and a counter for storing
resources and the corresponding handle numbers.  By changing to an idr
we can be safe from counter wrap-around giving two resources the same
handle.

Furthermore, the deallocation ioctls now check whether the resource to
be freed is of the intended type.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>

Some rework by Stefan R:
  - The idr API documentation says we get an ID within 0...0x7fffffff.
    Hence we can rest assured that idr handles fit into cdev handles.
  - Fix some races.  Add a client->in_shutdown flag for this purpose.
  - Add allocation retry to add_client_resource().
  - It is possible to use idr_for_each() in fw_device_op_release().
  - Fix ioctl_send_response() regression.
  - Small style changes.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:38 +01:00
Stefan Richter
97811e3473 firewire: cdev: fix race of fw_device_op_release with bus reset
Unlink the client from the fw_device earlier in order to prevent bus
reset events being added to client->event_list during shutdown.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:38 +01:00
Stefan Richter
1f3125af8e firewire: cdev: tcodes input validation
The behaviour of fw-transaction.c::fw_send_request is ill-defined for
any other tcodes than read/ write/ lock request tcodes.  Therefore
prevent requests with wrong tcodes from entering the transaction layer.

Maybe fw_send_request should check them itself, but I am not inclined to
change it and fw_fill_request from void-valued functions to ones which
return error codes and pass those up.  Besides, maybe fw_send_request is
going to support one more tcode than ioctl_send_request in the future
(TCODE_STREAM_DATA).

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:37 +01:00
Stefan Richter
d67cfb9613 firewire: convert client_list_lock to mutex
So far it is only taken in non-atomic contexts.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:37 +01:00
Jay Fenlason
cf417e5494 firewire: add a client_list_lock
This adds a client_list_lock, which only protects the device's
client_list, so that future versions of the driver can call code that
takes the card->lock while holding the client_list_lock.  Adding this
lock is much simpler than adding __ versions of all the functions that
the future version may need.  The one ordering issue is to make sure
code never takes the client_list_lock with card->lock held.  Since
client_list_lock is only used in three places, that isn't hard.

Signed-off-by: Jay Fenlason <fenlason@redhat.com>

Update fill_bus_reset_event() accordingly.  Include linux/spinlock.h.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:36 +01:00
David Moore
1aa292bb1c firewire: Include iso timestamp in headers when header_size > 4
Previously, when an iso context had header_size > 4, the iso header
(len/tag/channel/tcode/sy) was passed to userspace followed by quadlets
stripped from the payload.  This patch changes the behavior:
header_size = 8 now passes the header quadlet followed by the timestamp
quadlet.  When header_size > 8, quadlets are stripped from the payload.
The header_size = 4 case remains identical.

Since this alters the semantics of the API, the firewire API version
needs to be bumped concurrently with this change.

This change also refactors the header copying code slightly to be much
easier to read.

Signed-off-by: David Moore <dcm@acm.org>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-03-24 20:56:35 +01:00
Petr Vandrovec
b7479febde firewire: core: Remove card from list of cards when enable fails
Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

After a controller initialization failure, addition of another card got
stuck due to card_list corruption.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-02-01 11:17:24 +01:00
Stefan Richter
c8c4707cf7 firewire: sbp2: add workarounds for 2nd and 3rd generation iPods
According to https://bugs.launchpad.net/bugs/294391
  - 3rd generation iPods need the "fix capacity" workaround after all
    (apparently they crash after the last sector was accessed),
  - 2nd generation iPods need the "128 kB maximum request size"
    workaround.

Alas both iPod generations feature the same model ID in the config ROM,
hence we can only define a shared quirks list entry for them.  Luckily
the fix capacity workaround did not show a negative effect in Jarod's
tests with 2nd gen. iPod.

A side note:  Apple computers in target mode (or at least an x86 Mac
mini) don't have firmware_version and model_id, hence none of the iPod
quirks list entries is active for them.

Tested-by: Jarod Wilson <jarod@redhat.com>
Acked-by: Jarod Wilson <jarod@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-29 20:19:48 +01:00
Stefan Richter
5e2125677f firewire: sbp2: fix DMA mapping leak on the failure path
Reported-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
who also provided a first version of the fix.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-28 20:31:08 +01:00
Stefan Richter
f746072abc firewire: sbp2: define some magic numbers as macros
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-28 20:31:07 +01:00
Stefan Richter
a08e100aec firewire: sbp2: fix payload limit at S1600 and S3200
1394-2008 clause 16.3.4.1 (1394b-2002 clause 16.3.1.1) defines tighter
limits than 1394-2008 clause 6.2.2.3 (1394a-2000 clause 6.2.2.3).

Our previously too large limit doesn't matter though if the controller
reports its max_receive correctly.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-28 20:31:07 +01:00
Stefan Richter
e747a5c0be firewire: core: optimize card shutdown
This fixes a regression by "firewire: keep highlevel drivers attached
during brief connection loss":  There were 2 seconds unnecessary waiting
added to the shutdown procedure of each controller.

We use card->link as status flag to signal the device handler that there
is no use to wait for a come-back.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-24 20:40:12 +01:00
Stefan Richter
8b7b6afaa8 firewire: ohci: increase AT req. retries, fix ack_busy_X from Panasonic camcorders and others
Camcorders have a tendency to fail read requests to their config ROM and
write request to their FCP command register with ack_busy_X.  This has
become a problem with newer kernels and especially Panasonic camcorders,
causing AV/C in dvgrab and kino to fail.  Dvgrab for example frequently
logs "send oops"; kino reports loss of AV/C control.  I suspect that
lower CPU scheduling latencies in newer kernels made this issue more
prominent now.

According to
https://sourceforge.net/tracker/?func=detail&atid=114103&aid=2492640&group_id=14103
this can be fixed by configuring the FireWire controller for more
hardware retries for request transmission; these retries are evidently
more successful than libavc1394's own retry loop (typically 3 tries on
top of hardware retries).

Presumably the same issue has been reported at
https://bugzilla.redhat.com/show_bug.cgi?id=449252 and
https://bugzilla.redhat.com/show_bug.cgi?id=477279 .

In a quick test with a JVC camcorder (which didn't malfunction like the
reported camcorders), this change decreased the number of ack_busy_X
from 16 in three runs of dvgrab to 4 in three runs of the same capture
duration.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-24 11:17:27 +01:00
Stefan Richter
b006854955 firewire: ohci: change "context_stop: still active" log message
The present message is mostly just noise.  We only need to be notified
if the "active" flag does not go off before the retry loop terminates.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-24 11:17:27 +01:00
Stefan Richter
3d36a0df3b firewire: keep highlevel drivers attached during brief connection loss
There are situations when nodes vanish from the bus and come back
quickly thereafter:
  - When certain bus-powered hubs are plugged in,
  - when certain devices are plugged into 6-port hubs,
  - when certain disk enclosures are switched from self-power to bus
    power or vice versa and break the daisy chain during the transition,
  - when the user plugs a cable out and quickly plugs it back in, e.g.
    to reorder a daisy chain (works on Mac OS X if done quickly enough),
  - when certain hubs temporarily malfunction during high bus traffic.

Until now, firewire-core reported affected nodes as lost to the
highlevel drivers (firewire-sbp2 and userspace drivers).  We now delay
the destruction of device representations until after at least two
seconds after the last bus reset.  If a "new" device is detected in this
period whose bus information block and root directory header match that
of a device which is pending for deletion, we resurrect that device and
send update calls to highlevel drivers.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-20 19:29:52 +01:00
Stefan Richter
8cd0bbbdff firewire: unnecessary BM delay after generation rollover
Noticed by Jarod Wilson:  The bus manager work was unnecessarily delayed
each time the bus generation counter rolled over.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Jarod Wilson <jwilson@redhat.com>
2009-01-20 19:29:51 +01:00
Stefan Richter
a5c7f4710f firewire: insist on successive self ID complete events
The whole topology code only works if the old and new topologies which
are compared come from immediately successive self ID complete events.

If there happened bus resets without self ID complete events in the
meantime, or self ID complete events with invalid selfIDs, the topology
comparison could identify nodes wrongly, or more likely just corrupt
kernel memory or panic right away.

We now discard all nodes of the old topology and treat all current nodes
as new ones if the current self ID generation is not the previous one
plus 1.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Signed-off-by: Jarod Wilson <jwilson@redhat.com>
2009-01-20 19:29:51 +01:00
Stefan Richter
6230582320 firewire: core: fix sleep in atomic context due to driver core change
Due to commit 2831fe6f9c, "driver core:
create a private portion of struct device", device_initialize() can no
longer be called from atomic contexts.

We now defer it until after config ROM probing.  This requires changes
to the bus manager code because this may use a device before it was
probed.

Reported-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-09 23:22:32 +01:00
Stefan Richter
c8a12d45d5 firewire: reorder struct fw_card for better cache efficiency
topology_map is by far the largest member in struct fw_card.  Move it to
the very end of the struct so that card pointer dereferences have better
chances to hit the CPU cache.

This requires to increase the topology_map backing store to the size
specified in IEEE 1394, i.e. 256 rather than 255 quadlets.  Otherwise
the topology_map response handler may access invalid memory.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-04 23:50:38 +01:00
Stefan Richter
d6f95a3d14 firewire: fix resetting of bus manager retry counter
An earlier change, maybe long ago, removed the copying of self_id_count
into card->self_id_count.  Since then each bus reset cleared
card->bm_retries even when it shouldn't.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-04 23:50:38 +01:00
Jay Fenlason
0fa1986f3a firewire: improve refcounting of fw_card
Take a reference to the card whenever fw_card_bm_work() is scheduled on
that card and release it when the work is done.  This allows us to
remove the cancel_delayed_work_sync() in fw_core_remove_card().

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (patch update)
2009-01-04 23:50:37 +01:00
Jay Fenlason
2cc489c213 firewire: typo in comment
Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-04 23:50:37 +01:00
Stefan Richter
d6053e08f5 firewire: fix small memory leak at module removal
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-04 23:50:37 +01:00
Stefan Richter
621f6dd715 firewire: fw-sbp2: remove unnecessary locking
What was I thinking when I added sbp2_set_generation()?  Its locking did
nothing (except for implicitly providing the necessary barrier between
node IDs update and generation update).

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2009-01-04 23:50:36 +01:00
Stefan Richter
1d1dc5e83f firewire: fw-ohci: fix IOMMU resource exhaustion
There is a DMA map/ unmap imbalance whenever a block write request
packet is sent and then dequeued with ohci_cancel_packet.  The latter
may happen frequently if the AR resp tasklet is executed before the AT
req tasklet for the same transaction.

Add the missing dma_unmap_single.  This fixes
https://bugzilla.redhat.com/show_bug.cgi?id=475156

Reported-by: Emmanuel Kowalski
Tested-by: Emmanuel Kowalski
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2008-12-10 12:45:34 +01:00
Stefan Richter
031bb27c4b 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>
2008-11-25 21:38:31 +01:00
Kay Sievers
a1f64819fe 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>
2008-10-31 08:48:25 +01:00
Jay Fenlason
cd1f70fdb4 firewire: fw-sbp2: fix races
1: There is a small race between queue_delayed_work() and its
   corresponding kref_get().  Do the kref_get first, and _put it again
   if the queue_delayed_work() failed, so there is no chance of the
   kref going to zero while the work is scheduled.
2: An SBP2_LOGOUT_REQUEST could be sent out with a login_id full of
   garbage.  Initialize it to an invalid value so we can tell if we
   ever got a valid login_id.
3: The node ID and generation may have changed but the new values may
   not yet have been recorded in lu and tgt when the final logout is
   attempted.  Use the latest values from the device in
   sbp2_release_target().

Signed-off-by: Jay Fenlason <fenlason@redhat.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2008-10-26 10:27:01 +01:00
Stefan Richter
0dcfeb7e3c firewire: fw-sbp2: delay first login to avoid retries
This optimizes firewire-sbp2's device probe for the case that the local
node and the SBP-2 node were discovered at the same time.  In this case,
fw-core's bus management work and fw-sbp2's login and SCSI probe work
are scheduled in parallel (in the globally shared workqueue and in
fw-sbp2's workqueue, respectively).  The bus reset from fw-core may then
disturb and extremely delay the login and SCSI probe because the latter
fails with several command timeouts and retries and has to be retried
from scratch.

We avoid this particular situation of sbp2_login() and fw_card_bm_work()
running in parallel by delaying the first sbp2_login() a little bit.

This is meant to be a short-term fix for
https://bugzilla.redhat.com/show_bug.cgi?id=466679.  In the long run,
the SCSI probe, i.e. fw-sbp2's call of __scsi_add_device(), should be
parallelized with sbp2_reconnect().

Problem reported and fix tested and confirmed by Alex Kanavin.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
2008-10-26 10:27:01 +01:00