FUJITA Tomonori wrote:
On Wed, 26 Mar 2008 07:36:26 -0700
James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
On Wed, 2008-03-26 at 23:22 +0900, FUJITA Tomonori wrote:
On Sat, 22 Mar 2008 11:06:00 -0500
James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
On Tue, 2008-03-11 at 00:36 -0500, Mike Christie wrote:
Mike Christie wrote:
Pete Wyckoff wrote:
I think this used not to happen; not sure. But I changed two things
This most likely did not happen before 2.6.25-rc* or it broke in
slightly different ways, because iscsi used to try and do
echo 1 > /sys/block/sdX/device/delete
from userspace instead of calling scsi_remove_target from the kernel.
As you know around 2.6.21, the behavior of doing the echo to the delete
file changed due to a driver model and scsi change and that broke the
iscsi tools. The iscsi tools userspace removal was sort of hack in the
first place and was racey, so we switched to removing devices/target
like the FC class.
lately. 2.6.25-rc1 to -rc4 and fedora 8 iscsi-initiator-utils (865) to
fedora devel (868). Bidi and varlen patches always too.
I'll follow with some more variations on this theme. Looks like bsg
needs to protect more carefully against the device going away. Any
ideas how best to do this? What was the approach in sg?
I think sg is broken in similar ways. The iser guys have some tests
cases that have broken sg while IO is outstanding. I am ccing Erez.
Actually one of the problems looks a little different than some of the
problems hit with sg and are caused because we remove the bsg device too
soon. I think we want to wait until all the references from the
commands/requests are released. The attached patch (untested) moves the
bsg unreg call to the scsi device release fn.
Well, this fix is now upstream. However, it's causing all our
scsi_devices never to get released, which is a serious regression.
We're also doing spurious bsg_unregister_queue() for things that never
actually registered one (all scan devices that return DID_NO_CONNECT),
but bsg doesn't seem to be complaining about this.
The essence of the problem is that bsg_register_queue() takes a ref to
the sdev_gendev, so you can't move bsg_unregister_queue() into the
release function because nothing ever puts bsg's device ref and so
release is never called.
Options for fixing this before 2.6.25 are
1. revert the patch
2. Do an additional put for the bsg reference in
__scsi_remove_device (patch below). It's nasty but it preserves
the semantics and does what you want
After some investigation, this patch doesn't fix the bug that Pete
reported (I'll send a new patch shortly).
Can you revert the commit 4b6f5b3a993cbe34b4280f252bccc76967c185c8
instead of merging this?
Sure ... I didn't like the hack either. As long as iSCSI is fine with
the reversion it's the quickest way to fix the problem.
How about this? With the commit reversion, I confirmed that this patch
fixes the first bug that Pete reported:
http://marc.info/?l=linux-scsi&m=120508166505141&w=2
I suspect that this could fix the rest too.
=
From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>
Subject: [PATCH] bsg: takes a ref to struct device in fops->open
bsg_register_queue() takes a ref to struct device that a caller
passes. For example, it takes a ref to the sdev_gendev with scsi
devices. However, bsg doesn't takes a ref to it in fops->open. So
while an application opens a bsg device, the scsi device that the bsg
device holds can go away (bsg also takes a ref to a queue, but it
doesn't prevent the device from going away).
With this, bsg takes a ref to struct device in fops->open and frees it
in fops->release.
Note that bsg doesn't need to takes a ref to a queue for SCSI devices
at least. I think that it would be better to remove the code but I let
it alone for now.
I think you need the ref count on the queue for scsi_transport_sas
because it does not have the multi layer shutdown like how the
scsi_devices do. If a request was just added to the queue and
sas_smp_request was running, but then sas_bsg_remove is called the call
to blk_cleanup_queue would free the queue from under the request fn.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
