Skip to content

Commit

Permalink
Revert "fw_fcp: abort transaction at bus reset"
Browse files Browse the repository at this point in the history
This reverts commit 71d5953 since Linux
FireWire subsystem does not gurantee the order of events for bus reset
and asynchronous request subaction. It is likely to operate the
asynchronous request subaction wrongly due to generation of bus
topology different from the generation cached in user space.

Signed-off-by: Takashi Sakamoto <[email protected]>
  • Loading branch information
Takashi Sakamoto committed Aug 22, 2023
1 parent 05e95b9 commit 1548bb5
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 43 deletions.
43 changes: 3 additions & 40 deletions src/fw_fcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ G_DEFINE_QUARK(hinawa-fw-fcp-error-quark, hinawa_fw_fcp_error)
static const char *const local_err_msgs[] = {
[HINAWA_FW_FCP_ERROR_TIMEOUT] = "The transaction is canceled due to response timeout",
[HINAWA_FW_FCP_ERROR_LARGE_RESP] = "The size of response is larger than expected",
[HINAWA_FW_FCP_ERROR_ABORTED] = "The transaction is aborted due to bus reset",
};

#define generate_local_error(error, code) \
Expand Down Expand Up @@ -82,7 +81,6 @@ LIST_HEAD(waiter_entries, waiter);

typedef struct {
HinawaFwNode *node;
gulong bus_update_handler_id;
guint card_id;

struct waiter_entries transactions;
Expand Down Expand Up @@ -301,7 +299,6 @@ gboolean hinawa_fw_fcp_command(HinawaFwFcp *self, const guint8 *cmd, gsize cmd_s
enum waiter_state {
WAITER_STATE_PENDING = 0,
WAITER_STATE_RESPONDED,
WAITER_STATE_ABORTED,
};

struct waiter {
Expand Down Expand Up @@ -422,6 +419,7 @@ gboolean hinawa_fw_fcp_avc_transaction_with_tstamp(HinawaFwFcp *self,
}
deferred:
while (w.state == WAITER_STATE_PENDING) {
// NOTE: Timeout at bus-reset, illegally.
if (!g_cond_wait_until(&w.cond, &w.mutex, expiration))
break;
}
Expand Down Expand Up @@ -453,10 +451,6 @@ gboolean hinawa_fw_fcp_avc_transaction_with_tstamp(HinawaFwFcp *self,
tstamp[2] = w.tstamp;
}
break;
case WAITER_STATE_ABORTED:
generate_local_error(error, HINAWA_FW_FCP_ERROR_ABORTED);
result = FALSE;
break;
case WAITER_STATE_PENDING:
default:
generate_local_error(error, HINAWA_FW_FCP_ERROR_TIMEOUT);
Expand Down Expand Up @@ -613,8 +607,7 @@ static HinawaFwRcode handle_requested_signal(HinawaFwResp *resp, HinawaFwTcode t
NULL);
if (current.generation != record.generation)
node_history_insert_record(&priv->history, &current);

recorded = node_history_detect_record(&priv->history, &record);
recorded = !memcmp(&record, &current, sizeof(record));
}
node_history_unlock(&priv->history);

Expand All @@ -631,30 +624,6 @@ static HinawaFwRcode handle_requested_signal(HinawaFwResp *resp, HinawaFwTcode t
return HINAWA_FW_RCODE_COMPLETE;
}

static void handle_bus_update_signal(HinawaFwNode *node, HinawaFwFcp *self)
{
HinawaFwFcpPrivate *priv = hinawa_fw_fcp_get_instance_private(self);
struct waiter *w;
struct node_record record;

g_mutex_lock(&priv->transactions_mutex);

LIST_FOREACH(w, &priv->transactions, list) {
g_mutex_lock(&w->mutex);
w->state = WAITER_STATE_ABORTED;
g_cond_signal(&w->cond);
g_mutex_unlock(&w->mutex);
}

g_mutex_unlock(&priv->transactions_mutex);

g_object_get(node, "generation", &record.generation, "node-id", &record.src_node_id, NULL);

node_history_lock(&priv->history);
node_history_insert_record(&priv->history, &record);
node_history_unlock(&priv->history);
}

/**
* hinawa_fw_fcp_bind:
* @self: A [class@FwFcp].
Expand Down Expand Up @@ -698,9 +667,6 @@ gboolean hinawa_fw_fcp_bind(HinawaFwFcp *self, HinawaFwNode *node, GError **erro
node_history_insert_record(&priv->history, &record);
node_history_unlock(&priv->history);

priv->bus_update_handler_id =
g_signal_connect(node, "bus-update",
G_CALLBACK(handle_bus_update_signal), self);
priv->node = g_object_ref(node);
}

Expand All @@ -725,7 +691,6 @@ void hinawa_fw_fcp_unbind(HinawaFwFcp *self)

if (priv->node != NULL) {
hinawa_fw_resp_release(HINAWA_FW_RESP(self));
g_signal_handler_disconnect(priv->node, priv->bus_update_handler_id);

g_object_unref(priv->node);
priv->node = NULL;
Expand All @@ -736,10 +701,8 @@ void hinawa_fw_fcp_unbind(HinawaFwFcp *self)
LIST_FOREACH(w, &priv->transactions, list) {
g_mutex_lock(&w->mutex);

if (w->state == WAITER_STATE_PENDING) {
w->state = WAITER_STATE_ABORTED;
if (w->state == WAITER_STATE_PENDING)
g_cond_signal(&w->cond);
}

g_mutex_unlock(&w->mutex);
}
Expand Down
2 changes: 0 additions & 2 deletions src/hinawa_enum_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ typedef enum {
* HinawaFwFcpError:
* @HINAWA_FW_FCP_ERROR_TIMEOUT: The transaction is canceled due to response timeout.
* @HINAWA_FW_FCP_ERROR_LARGE_RESP: The size of response is larger than expected.
* @HINAWA_FW_FCP_ERROR_ABORTED: The transaction is aborted due to bus reset.
*
* A set of error code for [[email protected]] with domain which equals to Hinawa.FwFcpError.
*
Expand All @@ -162,7 +161,6 @@ typedef enum {
typedef enum {
HINAWA_FW_FCP_ERROR_TIMEOUT,
HINAWA_FW_FCP_ERROR_LARGE_RESP,
HINAWA_FW_FCP_ERROR_ABORTED,
} HinawaFwFcpError;

G_END_DECLS
Expand Down
1 change: 0 additions & 1 deletion tests/hinawa-enum
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ fw_resp_error_enumerations = (
fw_fcp_error_enumerators = (
'TIMEOUT',
'LARGE_RESP',
'ABORTED',
)

types = {
Expand Down

0 comments on commit 1548bb5

Please sign in to comment.