On 4/26/23 10:35 AM, Mike Christie wrote:
> On 4/17/23 12:17 PM, Maurizio Lombardi wrote:
>> static void iscsi_target_do_login_rx(struct work_struct *work)
>> {
>> struct iscsit_conn *conn = container_of(work,
>> @@ -562,12 +543,15 @@ static void iscsi_target_do_login_rx(struct work_struct *work)
>> struct iscsi_np *np = login->np;
>> struct iscsi_portal_group *tpg = conn->tpg;
>> struct iscsi_tpg_np *tpg_np = conn->tpg_np;
>> - struct conn_timeout timeout;
>> int rc, zero_tsih = login->zero_tsih;
>> bool state;
>>
>> pr_debug("entering iscsi_target_do_login_rx, conn: %p, %s:%d\n",
>> conn, current->comm, current->pid);
>> +
>> + spin_lock(&conn->login_worker_lock);
>> + set_bit(LOGIN_FLAGS_WORKER_RUNNING, &conn->login_flags);
>> + spin_unlock(&conn->login_worker_lock);
>
>
> I think the locking bh use is reversed.
Ignore the locking comment like here and below. I see there are 2 locks.
>> +
>> + if (iscsi_target_sk_check_and_clear(conn, LOGIN_FLAGS_INITIAL_PDU))
>> + ret = -1;
>> + else if (!test_bit(LOGIN_FLAGS_WORKER_RUNNING, &conn->login_flags)) {
>> + if (iscsit_set_login_timer_kworker(conn, NULL) < 0) {
>
> I think this will deadlock because iscsit_set_login_timer_kworker tries to take the
> login_worker_lock we took above.
