iscsi_target: race condition on shutdown

PATCH In review Hannes Reinecke 2 participants 7 messages Dec 5, 2013 – Dec 18, 2013

Subsystems (2)

SCSI SCSI target

lore

Share & tools

lore

Hannes Reinecke

author

2013-12-05 13:54 UTC #0 lore raw

PATCH 1/1 iscsi_target: race condition on shutdown

When shutting down a target there is a race condition between iscsit_del_np() and __iscsi_target_login_thread(). The latter sets the thread pointer to NULL, and the former tries to issue kthread_stop() on that pointer without any synchronization.

This patchs adds proper synchronization pointer between those calls to ensure that a) the thread is correctly terminate and
b) kthread_stop() isn't called with a NULL pointer.

In the long run iscsi_target_login_thread() should be converted into a workqueue.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/target/iscsi/iscsi_target.c       | 12 +++++++++---
 drivers/target/iscsi/iscsi_target_login.c |  9 ++++++---
 2 files changed, 15 insertions(+), 6 deletions(-)

drivers/target/iscsi/iscsi_target.c +9 -3

index bf76fc4..c7bf3c9 100644
@@ -457,15 +457,21 @@ int iscsit_del_np(struct iscsi_np *np)
 	}
 	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
 	spin_unlock_bh(&np->np_thread_lock);
-
-	if (np->np_thread) {
+	/* Give __iscsi_target_login_thread() a chance to run */
+	schedule();
+	spin_lock_bh(&np->np_thread_lock);
+	if ((np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN)
+	    && np->np_thread) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		spin_unlock_bh(&np->np_thread_lock);
 		/*
 		 * We need to send the signal to wakeup Linux/Net
 		 * which may be sleeping in sock_accept()..
 		 */
 		send_sig(SIGINT, np->np_thread, 1);
 		kthread_stop(np->np_thread);
-	}
+	} else
+		spin_unlock_bh(&np->np_thread_lock);
 
 	np->np_transport->iscsit_free_np(np);

drivers/target/iscsi/iscsi_target_login.c +6 -3

index 4eb93b2..b375d26 100644
@@ -1405,7 +1405,8 @@ out:
 	stop = kthread_should_stop();
 	if (!stop && signal_pending(current)) {
 		spin_lock_bh(&np->np_thread_lock);
-		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
+		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN ||
+			np->np_thread_state == ISCSI_NP_THREAD_EXIT);
 		spin_unlock_bh(&np->np_thread_lock);
 	}
 	/* Wait for another socket.. */

@@ -1414,8 +1415,10 @@ out:
 exit:
 	iscsi_stop_login_thread_timer(np);
 	spin_lock_bh(&np->np_thread_lock);
-	np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-	np->np_thread = NULL;
+	if (np->np_thread_state != ISCSI_NP_THREAD_EXIT) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		np->np_thread = NULL;
+	}
 	spin_unlock_bh(&np->np_thread_lock);
 
 	return 0;

Signature · 1 line

1.7.12.4

Nicholas A. Bellinger

2013-12-11 23:44 UTC #1 lore raw

Hi Hannes,

Btw, apologies for the delayed response on this.. Comments are below.

Quoted 65 lines · Hannes Reinecke

This patchs adds proper synchronization pointer between those calls to ensure that a) the thread is correctly terminate and
b) kthread_stop() isn't called with a NULL pointer.

In the long run iscsi_target_login_thread() should be converted into a workqueue.

Signed-off-by Hannes Reinecke <[email protected]>

---
 drivers/target/iscsi/iscsi_target.c       | 12 +++++++++---
 drivers/target/iscsi/iscsi_target_login.c |  9 ++++++---
 2 files changed, 15 insertions(+), 6 deletions(-)

drivers/target/iscsi/iscsi_target.c +9 -3

index bf76fc4..c7bf3c9 100644
@@ -457,15 +457,21 @@ int iscsit_del_np(struct iscsi_np *np)
 	}
 	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
 	spin_unlock_bh(&np->np_thread_lock);
-
-	if (np->np_thread) {
+	/* Give __iscsi_target_login_thread() a chance to run */
+	schedule();
+	spin_lock_bh(&np->np_thread_lock);
+	if ((np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN)
+	    && np->np_thread) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		spin_unlock_bh(&np->np_thread_lock);
 		/*
 		 * We need to send the signal to wakeup Linux/Net
 		 * which may be sleeping in sock_accept()..
 		 */
 		send_sig(SIGINT, np->np_thread, 1);
 		kthread_stop(np->np_thread);
-	}
+	} else
+		spin_unlock_bh(&np->np_thread_lock);
 
 	np->np_transport->iscsit_free_np(np);

drivers/target/iscsi/iscsi_target_login.c +2 -1

index 4eb93b2..b375d26 100644
@@ -1405,7 +1405,8 @@ out:
 	stop = kthread_should_stop();
 	if (!stop && signal_pending(current)) {
 		spin_lock_bh(&np->np_thread_lock);
-		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
+		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN ||
+			np->np_thread_state == ISCSI_NP_THREAD_EXIT);
 		spin_unlock_bh(&np->np_thread_lock);
 	}
 	/* Wait for another socket.. */

@@ -1414,8 +1415,10 @@ out:
 exit:
 	iscsi_stop_login_thread_timer(np);

drivers/target/iscsi/iscsi_target_login.c out: · line 1424

Quoted 10 lines

drivers/target/iscsi/iscsi_target_login.c +4 -2

 	spin_lock_bh(&np->np_thread_lock);
-	np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-	np->np_thread = NULL;
+	if (np->np_thread_state != ISCSI_NP_THREAD_EXIT) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		np->np_thread = NULL;
+	}
 	spin_unlock_bh(&np->np_thread_lock);
 
 	return 0;

I'm not sure this extra logic is necessary. How about just clearing np->np_thread in iscsit_del_np instead..?

Care to verify on your side with the following patch..?

--nab

drivers/target/iscsi/iscsi_target.c +1 -0

index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);

drivers/target/iscsi/iscsi_target_login.c +0 -1

index 4eb93b2..6ab43b6 100644
@@ -1415,7 +1415,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;

Hannes Reinecke

author

2013-12-12 07:18 UTC #2 lore raw

Quoted 107 lines · Nicholas A. Bellinger

Hi Hannes,
Btw, apologies for the delayed response on this.. Comments are below.
Quoted 65 lines · Hannes Reinecke
When shutting down a target there is a race condition between iscsit_del_np() and __iscsi_target_login_thread(). The latter sets the thread pointer to NULL, and the former tries to issue kthread_stop() on that pointer without any synchronization.
This patchs adds proper synchronization pointer between those calls to ensure that a) the thread is correctly terminate and
b) kthread_stop() isn't called with a NULL pointer.
In the long run iscsi_target_login_thread() should be converted into a workqueue.
Signed-off-by Hannes Reinecke <[email protected]>
---
 drivers/target/iscsi/iscsi_target.c       | 12 +++++++++---
 drivers/target/iscsi/iscsi_target_login.c |  9 ++++++---
 2 files changed, 15 insertions(+), 6 deletions(-)
drivers/target/iscsi/iscsi_target.c +9 -3
index bf76fc4..c7bf3c9 100644
@@ -457,15 +457,21 @@ int iscsit_del_np(struct iscsi_np *np)
 	}
 	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
 	spin_unlock_bh(&np->np_thread_lock);
-
-	if (np->np_thread) {
+	/* Give __iscsi_target_login_thread() a chance to run */
+	schedule();
+	spin_lock_bh(&np->np_thread_lock);
+	if ((np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN)
+	    && np->np_thread) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		spin_unlock_bh(&np->np_thread_lock);
 		/*
 		 * We need to send the signal to wakeup Linux/Net
 		 * which may be sleeping in sock_accept()..
 		 */
 		send_sig(SIGINT, np->np_thread, 1);
 		kthread_stop(np->np_thread);
-	}
+	} else
+		spin_unlock_bh(&np->np_thread_lock);
 
 	np->np_transport->iscsit_free_np(np);
 
drivers/target/iscsi/iscsi_target_login.c +2 -1
index 4eb93b2..b375d26 100644
@@ -1405,7 +1405,8 @@ out:
 	stop = kthread_should_stop();
 	if (!stop && signal_pending(current)) {
 		spin_lock_bh(&np->np_thread_lock);
-		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
+		stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN ||
+			np->np_thread_state == ISCSI_NP_THREAD_EXIT);
 		spin_unlock_bh(&np->np_thread_lock);
 	}
 	/* Wait for another socket.. */
@@ -1414,8 +1415,10 @@ out:
 exit:
 	iscsi_stop_login_thread_timer(np);
drivers/target/iscsi/iscsi_target_login.c out: · line 1424
Quoted 10 lines
drivers/target/iscsi/iscsi_target_login.c +4 -2
 	spin_lock_bh(&np->np_thread_lock);
-	np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-	np->np_thread = NULL;
+	if (np->np_thread_state != ISCSI_NP_THREAD_EXIT) {
+		np->np_thread_state = ISCSI_NP_THREAD_EXIT;
+		np->np_thread = NULL;
+	}
 	spin_unlock_bh(&np->np_thread_lock);
 
 	return 0;
I'm not sure this extra logic is necessary. How about just clearing np->np_thread in iscsit_del_np instead..?
Care to verify on your side with the following patch..?
--nab
drivers/target/iscsi/iscsi_target.c +1 -0
index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);
drivers/target/iscsi/iscsi_target_login.c +0 -0
index 4eb93b2..6ab43b6 100644

drivers/target/iscsi/iscsi_target_login.c exit: · line 1420

Quoted 7 lines

drivers/target/iscsi/iscsi_target_login.c +0 -1

@@ -1415,7 +1415,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;

The problem here is that 'kthread_stop()' is supposed to be called with a _valid_ task structure.

There is this race window:

	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
	spin_unlock_bh(&np->np_thread_lock);
here ->
	if (np->np_thread) {
		/*

If the login thread exits before we evaluate 'np->np_thread' the pointer is stale and kthread_stop will be called with an invalid task structure.

So at the very least we need to check the thread_state before evaluating 'np->np_thread' (which will evaluate to 'true' anyway if we were to follow up with your patch).
But in doing so we would need to protect is by the thread_lock to synchronize the state.
And we'll end up with quite the same patch as I've send originally.

In fact, it was an invalid call to kthread_stop() which triggered the whole patch in the first place :-)

I would love to be proven wrong, as I'm not keen on the 'schedule()' in there. But I fail to see another way out here, short of converting the entire kthread into a workqueue item ...

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Nicholas A. Bellinger

2013-12-12 08:04 UTC #3 lore raw

Quoted 1 line · Hannes Reinecke

On 12/12/2013 12:44 AM, Nicholas A. Bellinger wrote:

<SNIP>

Quoted 55 lines

Quoted 26 lines
I'm not sure this extra logic is necessary. How about just clearing np->np_thread in iscsit_del_np instead..?
Care to verify on your side with the following patch..?
--nab
drivers/target/iscsi/iscsi_target.c +1 -0
index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);
drivers/target/iscsi/iscsi_target_login.c +0 -0
index 4eb93b2..6ab43b6 100644
drivers/target/iscsi/iscsi_target_login.c exit: · line 1420
Quoted 7 lines
drivers/target/iscsi/iscsi_target_login.c +0 -1
@@ -1415,7 +1415,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;
The problem here is that 'kthread_stop()' is supposed to be called with a _valid_ task structure.
There is this race window:
	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
	spin_unlock_bh(&np->np_thread_lock);
here ->
	if (np->np_thread) {
		/*
If the login thread exits before we evaluate 'np->np_thread' the pointer is stale and kthread_stop will be called with an invalid task structure.
So at the very least we need to check the thread_state before evaluating 'np->np_thread' (which will evaluate to 'true' anyway if we were to follow up with your patch).
But in doing so we would need to protect is by the thread_lock to synchronize the state.
And we'll end up with quite the same patch as I've send originally.

Quoted 3 lines

In fact, it was an invalid call to kthread_stop() which triggered the whole patch in the first place :-)

Mmmm, point taken..

Quoted 3 lines

I would love to be proven wrong, as I'm not keen on the 'schedule()' in there. But I fail to see another way out here, short of converting the entire kthread into a workqueue item ...

Thinking about this a bit more, I think the pre-kthread API np_thead_state check to exit at the out: label in __iscsi_target_login_thread() is the culprit..

How about following to only exit when iscsit_del_np() -> kthread_stop() has been called, and kthread_should_stop() is true..?

--nab

drivers/target/iscsi/iscsi_target.c +1 -0

index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);

drivers/target/iscsi/iscsi_target_login.c +0 -6

index 4eb93b2..e29279e 100644
@@ -1403,11 +1403,6 @@ old_sess_out:
 
 out:
        stop = kthread_should_stop();
-       if (!stop && signal_pending(current)) {
-               spin_lock_bh(&np->np_thread_lock);
-               stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
-               spin_unlock_bh(&np->np_thread_lock);
-       }
        /* Wait for another socket.. */
        if (!stop)
                return 1;

@@ -1415,7 +1410,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;

Hannes Reinecke

author

2013-12-12 08:05 UTC #4 lore raw

Quoted 1 line · Nicholas A. Bellinger

On Thu, 2013-12-12 at 08:18 +0100, Hannes Reinecke wrote:

[ .. ]

Quoted 72 lines

Quoted 22 lines
The problem here is that 'kthread_stop()' is supposed to be called with a _valid_ task structure.
There is this race window:
	np->np_thread_state = ISCSI_NP_THREAD_SHUTDOWN;
	spin_unlock_bh(&np->np_thread_lock);
here ->
	if (np->np_thread) {
		/*
If the login thread exits before we evaluate 'np->np_thread' the pointer is stale and kthread_stop will be called with an invalid task structure.
So at the very least we need to check the thread_state before evaluating 'np->np_thread' (which will evaluate to 'true' anyway if we were to follow up with your patch).
But in doing so we would need to protect is by the thread_lock to synchronize the state.
And we'll end up with quite the same patch as I've send originally.
Quoted 3 lines
In fact, it was an invalid call to kthread_stop() which triggered the whole patch in the first place :-)
Mmmm, point taken..
Quoted 3 lines
I would love to be proven wrong, as I'm not keen on the 'schedule()' in there. But I fail to see another way out here, short of converting the entire kthread into a workqueue item ...
Thinking about this a bit more, I think the pre-kthread API np_thead_state check to exit at the out: label in __iscsi_target_login_thread() is the culprit..
How about following to only exit when iscsit_del_np() -> kthread_stop() has been called, and kthread_should_stop() is true..?
--nab
drivers/target/iscsi/iscsi_target.c +1 -0
index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);
drivers/target/iscsi/iscsi_target_login.c +0 -5
index 4eb93b2..e29279e 100644
@@ -1403,11 +1403,6 @@ old_sess_out:
 
 out:
        stop = kthread_should_stop();
-       if (!stop && signal_pending(current)) {
-               spin_lock_bh(&np->np_thread_lock);
-               stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
-               spin_unlock_bh(&np->np_thread_lock);
-       }
        /* Wait for another socket.. */
        if (!stop)
                return 1;

drivers/target/iscsi/iscsi_target_login.c exit: · line 1415

Quoted 7 lines

drivers/target/iscsi/iscsi_target_login.c +0 -1

@@ -1415,7 +1410,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;

Yes. Far better.

I'll give it a spin.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Nicholas A. Bellinger

2013-12-16 19:49 UTC #5 lore raw

Hey Hannes,

Quoted 3 lines · Hannes Reinecke

Quoted 1 line · Nicholas A. Bellinger
On Thu, 2013-12-12 at 08:18 +0100, Hannes Reinecke wrote:
[ .. ]

<SNIP>

Quoted 50 lines

Mmmm, point taken..

Quoted 3 lines

I would love to be proven wrong, as I'm not keen on the 'schedule()' in there. But I fail to see another way out here, short of converting the entire kthread into a workqueue item ...

Thinking about this a bit more, I think the pre-kthread API np_thead_state check to exit at the out: label in __iscsi_target_login_thread() is the culprit..

How about following to only exit when iscsit_del_np() -> kthread_stop() has been called, and kthread_should_stop() is true..?

--nab

drivers/target/iscsi/iscsi_target.c +1 -0

index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);

drivers/target/iscsi/iscsi_target_login.c +0 -6

index 4eb93b2..e29279e 100644
@@ -1403,11 +1403,6 @@ old_sess_out:
 
 out:
        stop = kthread_should_stop();
-       if (!stop && signal_pending(current)) {
-               spin_lock_bh(&np->np_thread_lock);
-               stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
-               spin_unlock_bh(&np->np_thread_lock);
-       }
        /* Wait for another socket.. */
        if (!stop)
                return 1;

@@ -1415,7 +1410,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);

Quoted 7 lines

drivers/target/iscsi/iscsi_target_login.c exit: · line 1415
Quoted 3 lines
drivers/target/iscsi/iscsi_target_login.c +0 -0
        return 0;
Yes. Far better.
I'll give it a spin.

Any chance to confirm this patch..?

I'd like to include this in the -rc5 PULL request over the next days.

--nab

Nicholas A. Bellinger

2013-12-18 02:27 UTC #6 lore raw

Hey Hannes,

Quoted 64 lines · Nicholas A. Bellinger

Hey Hannes,
Quoted 3 lines · Hannes Reinecke
Quoted 1 line · Nicholas A. Bellinger
On Thu, 2013-12-12 at 08:18 +0100, Hannes Reinecke wrote:
[ .. ]
<SNIP>
Quoted 55 lines
Quoted 46 lines
Mmmm, point taken..
Quoted 3 lines
I would love to be proven wrong, as I'm not keen on the 'schedule()' in there. But I fail to see another way out here, short of converting the entire kthread into a workqueue item ...
Thinking about this a bit more, I think the pre-kthread API np_thead_state check to exit at the out: label in __iscsi_target_login_thread() is the culprit..
How about following to only exit when iscsit_del_np() -> kthread_stop() has been called, and kthread_should_stop() is true..?
--nab
drivers/target/iscsi/iscsi_target.c +1 -0
index 02182ab..0086719 100644
@@ -465,6 +465,7 @@ int iscsit_del_np(struct iscsi_np *np)
                 */
                send_sig(SIGINT, np->np_thread, 1);
                kthread_stop(np->np_thread);
+               np->np_thread = NULL;
        }
 
        np->np_transport->iscsit_free_np(np);
drivers/target/iscsi/iscsi_target_login.c +0 -5
index 4eb93b2..e29279e 100644
@@ -1403,11 +1403,6 @@ old_sess_out:
 
 out:
        stop = kthread_should_stop();
-       if (!stop && signal_pending(current)) {
-               spin_lock_bh(&np->np_thread_lock);
-               stop = (np->np_thread_state == ISCSI_NP_THREAD_SHUTDOWN);
-               spin_unlock_bh(&np->np_thread_lock);
-       }
        /* Wait for another socket.. */
        if (!stop)
                return 1;
drivers/target/iscsi/iscsi_target_login.c exit: · line 1415
Quoted 7 lines
drivers/target/iscsi/iscsi_target_login.c +0 -1
@@ -1415,7 +1410,6 @@ exit:
        iscsi_stop_login_thread_timer(np);
        spin_lock_bh(&np->np_thread_lock);
        np->np_thread_state = ISCSI_NP_THREAD_EXIT;
-       np->np_thread = NULL;
        spin_unlock_bh(&np->np_thread_lock);
 
        return 0;
Yes. Far better.

Quoted 6 lines

Quoted 2 lines
I'll give it a spin.
Any chance to confirm this patch..?
I'd like to include this in the -rc5 PULL request over the next days.

Any testing feedback on this..? Waiting for your Reviewed-by + Tested-by before pushing this one.

--nab