linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zhong Jinghua <zhongjinghua@huawei.com>
To: <gregkh@linuxfoundation.org>, <jejb@linux.ibm.com>,
	<martin.petersen@oracle.com>, <hare@suse.de>,
	<bvanassche@acm.org>, <emilne@redhat.com>
Cc: <linux-kernel@vger.kernel.org>, <linux-scsi@vger.kernel.org>,
	<zhongjinghua@huawei.com>, <yi.zhang@huawei.com>,
	<yukuai3@huawei.com>
Subject: [PATCH-next v2 2/2] scsi: fix iscsi rescan fails to create block device
Date: Sat, 28 Jan 2023 17:41:46 +0800	[thread overview]
Message-ID: <20230128094146.205858-3-zhongjinghua@huawei.com> (raw)
In-Reply-To: <20230128094146.205858-1-zhongjinghua@huawei.com>

When the three iscsi operations delete, logout, and rescan are concurrent
at the same time, there is a probability of failure to add disk through
device_add_disk(). The concurrent process is as follows:

T0: scan host // echo 1 > /sys/devices/platform/host1/scsi_host/host1/scan
T1: delete target // echo 1 > /sys/devices/platform/host1/session1/target1:0:0/1:0:0:1/delete
T2: logout // iscsiadm -m node --login
T3: T2 scsi_queue_work
T4: T0 bus_probe_device

T0                          T1                     T2                     T3
scsi_scan_target
 mutex_lock(&shost->scan_mutex);
  __scsi_scan_target
   scsi_report_lun_scan
    scsi_add_lun
     scsi_sysfs_add_sdev
      device_add
       kobject_add
       //create session1/target1:0:0/1:0:0:1/
       ...
       bus_probe_device
       // Create block asynchronously
 mutex_unlock(&shost->scan_mutex);
                       sdev_store_delete
                        scsi_remove_device
                         device_remove_file
                          mutex_lock(scan_mutex)
                           __scsi_remove_device
                            res = scsi_device_set_state(sdev, SDEV_CANCEL)
                                             iscsi_if_recv_msg
                                              scsi_queue_work
                                                                 __iscsi_unbind_session
                                                                 session->target_id = ISCSI_MAX_TARGET
                                                                   __scsi_remove_target
                                                                   sdev->sdev_state == SDEV_CANCEL
                                                                   continue;
                                                                   // end, No delete kobject 1:0:0:1
                                             iscsi_if_recv_msg
                                              transport->destroy_session(session)
                                               __iscsi_destroy_session
                                               iscsi_session_teardown
                                                iscsi_remove_session
                                                 __iscsi_unbind_session
                                                  iscsi_session_event
                                                 device_del
                                                 // delete session
T4:
// create the block, its parent is 1:0:0:1
// If kobject 1:0:0:1 does not exist, it won't go down
__device_attach_async_helper
 device_lock
 ...
 __device_attach_driver
  driver_probe_device
   really_probe
    sd_probe
     device_add_disk
      register_disk
       device_add
      // error

The block is created after the seesion is deleted.
When T2 deletes the session, it will mark block'parent 1:0:01 as unusable:
T2
device_del
 kobject_del
  sysfs_remove_dir
   __kernfs_remove
   // Mark the children under the session as unusable
    while ((pos = kernfs_next_descendant_post(pos, kn)))
		if (kernfs_active(pos))
			atomic_add(KN_DEACTIVATED_BIAS, &pos->active);

Then, create the block:
T4
device_add
 kobject_add
  kobject_add_varg
   kobject_add_internal
    create_dir
     sysfs_create_dir_ns
      kernfs_create_dir_ns
       kernfs_add_one
        if ((parent->flags & KERNFS_ACTIVATED) && !kernfs_active(parent))
		goto out_unlock;
		// return error

This error will cause a warning:
kobject_add_internal failed for block (error: -2 parent: 1:0:0:1).
In the lower version (such as 5.10), there is no corresponding error handling, continuing
to go down will trigger a kernel panic, so cc stable.

Therefore, creating the block should not be done after deleting the session.
More practically, we should ensure that the target under the session is deleted first,
and then the session is deleted. In this way, there are two possibilities:

1) if the process(T1) of deleting the target execute first, it will grab the device_lock(),
and the process(T4) of creating the block will wait for the deletion to complete.
Then, block's parent 1:0:0:1 has been deleted, it won't go down.

2) if the process(T4) of creating block execute first, it will grab the device_lock(),
and the process(T1) of deleting the target will wait for the creation block to complete.
Then, the process(T2) of deleting the session should need wait for the deletion to complete.

Fix it by removing the judgment of state equal to SDEV_CANCEL in
__scsi_remove_target() to ensure the order of deletion. Then, it will wait for
T1's mutex_lock(scan_mutex) and device_del() in __scsi_remove_device() will wait for
T4's device_lock(dev).
But we found that such a fix would cause the previous problem:
commit 81b6c9998979 ("scsi: core: check for device state in __scsi_remove_target()").
So we use get_device_unless_zero() instead of get_devcie() to fix the previous problem.

Fixes: 81b6c9998979 ("scsi: core: check for device state in __scsi_remove_target()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
---
 drivers/scsi/scsi_sysfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index cac7c902cf70..a22109cdb8ef 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1535,9 +1535,7 @@ static void __scsi_remove_target(struct scsi_target *starget)
 		if (sdev->channel != starget->channel ||
 		    sdev->id != starget->id)
 			continue;
-		if (sdev->sdev_state == SDEV_DEL ||
-		    sdev->sdev_state == SDEV_CANCEL ||
-		    !get_device(&sdev->sdev_gendev))
+		if (!get_device_unless_zero(&sdev->sdev_gendev))
 			continue;
 		spin_unlock_irqrestore(shost->host_lock, flags);
 		scsi_remove_device(sdev);
-- 
2.31.1


  parent reply	other threads:[~2023-01-28  9:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-28  9:41 [PATCH-next v2 0/2] scsi, driver core: fix iscsi rescan fails to create block device Zhong Jinghua
2023-01-28  9:41 ` [PATCH-next v2 1/2] driver core: introduce get_device_unless_zero() Zhong Jinghua
2023-01-28 10:43   ` Greg KH
2023-01-28  9:41 ` Zhong Jinghua [this message]
2023-01-28 10:45   ` [PATCH-next v2 2/2] scsi: fix iscsi rescan fails to create block device Greg KH
2023-01-29  1:13     ` Yu Kuai
2023-01-29  6:46       ` Greg KH
2023-01-29  6:55         ` Yu Kuai
2023-01-29 17:30   ` James Bottomley
2023-01-30  3:07     ` Yu Kuai
2023-01-30  3:29       ` James Bottomley
2023-01-30  3:46         ` Yu Kuai
2023-01-30 13:17           ` James Bottomley
2023-01-31  1:43             ` Yu Kuai
2023-01-31  3:25               ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230128094146.205858-3-zhongjinghua@huawei.com \
    --to=zhongjinghua@huawei.com \
    --cc=bvanassche@acm.org \
    --cc=emilne@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hare@suse.de \
    --cc=jejb@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=yi.zhang@huawei.com \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).