All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"songliubraving@fb.com" <songliubraving@fb.com>
Cc: "hch@infradead.org" <hch@infradead.org>
Subject: Re: [RFC] scsi: reduce protection of scan_mutex in scsi_remove_device
Date: Tue, 25 Apr 2017 17:23:49 +0000	[thread overview]
Message-ID: <1493141028.2628.4.camel@sandisk.com> (raw)
In-Reply-To: <20170421211302.2667649-1-songliubraving@fb.com>

On Fri, 2017-04-21 at 14:13 -0700, Song Liu wrote:
> When a device is deleted through sysfs handle "delete", the code
> locks shost->scan_mutex. If multiple devices are deleted at the
> same time, these deletes will be handled in series.
> 
> On the other hand, some devices do long latency IO during deletion,
> for example, sd_shutdown() may do sync cache and/or start_stop.
> It is not necessary for these commands to run in series.
> 
> To reduce latency of parallel "delete" requests, this patch reduces
> the protection of scan_mutex. The only function with Scsi_Host
> called in __scsi_remove_device() is the optional slave_destroy().
> Therefore, the protection of scan_mutex is only necessary for this
> function.

Hello Song,

What you wrote above is wrong. It is necessary to serialize SCSI
scanning against SCSI device removal. That is why scan_mutex is held
around the __scsi_remove_device() call. When adding a LUN the following
(and several other) actions are performed:
* Allocation of memory for struct scsi_device.
* Allocation of a block layer request queue.
* Initialization of the .sdev_gendev and .sdev_dev device structures.
* Association of the transport layer driver with the SCSI device.
* Association of a device handler with the SCSI device (e.g. ALUA).
* Making the .sdev_gendev and .sdev_dev devices visible in sysfs.
* Making the transport layer attributes visible in sysfs.
* Creating a bsg device node in sysfs.
* Association of an upper layer driver
(e.g. sd or sr) with the SCSI LUN.

Removal of a LUN means undoing all of the above. If adding and removing
a LUN would not be not serialized then there would be a risk that removal
and immediate reregistration of a LUN will fail due to the reregistration
process trying to add sysfs attributes that were not yet removed by the
removal step.

Bart.

  parent reply	other threads:[~2017-04-25 17:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-21 21:13 [RFC] scsi: reduce protection of scan_mutex in scsi_remove_device Song Liu
2017-04-21 21:17 ` Bart Van Assche
2017-04-21 22:20   ` Song Liu
2017-04-21 21:20 ` Bart Van Assche
2017-04-21 22:31   ` Song Liu
2017-04-25 20:59     ` Bart Van Assche
2017-04-25 21:29       ` Song Liu
2017-04-24 15:28 ` Christoph Hellwig
2017-04-25 17:23 ` Bart Van Assche [this message]
2017-04-25 17:42   ` Song Liu
2017-04-25 17:52     ` Bart Van Assche
2017-04-25 21:17       ` Song Liu
2017-04-25 22:17         ` Bart Van Assche
2017-04-26  0:41           ` Song Liu

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=1493141028.2628.4.camel@sandisk.com \
    --to=bart.vanassche@sandisk.com \
    --cc=hch@infradead.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=songliubraving@fb.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.