All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Mansfield <patmans@us.ibm.com>
To: Tejun Heo <htejun@gmail.com>
Cc: James Bottomley <James.Bottomley@SteelEye.com>,
	Jens Axboe <axboe@suse.de>,
	SCSI Mailing List <linux-scsi@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH scsi-misc-2.6 07/08] scsi: remove bogus	{get|put}_device() calls
Date: Tue, 29 Mar 2005 09:02:01 -0800	[thread overview]
Message-ID: <20050329170201.GA6787@us.ibm.com> (raw)
In-Reply-To: <42413336.2010004@gmail.com>

On Wed, Mar 23, 2005 at 06:13:26PM +0900, Tejun Heo wrote:
>  Hi,
> 
> James Bottomley wrote:
> >On Wed, 2005-03-23 at 11:14 +0900, Tejun Heo wrote:
> >
> >>	So, basically, SCSI high-level object (scsi_disk) and
> >>	mid-level object (scsi_device) are reference counted by users,
> >>	not the requests they submit.  Reference count cannot go zero
> >>	with active users and users cannot access the object once the
> >>	reference count reaches zero.
> >
> >
> >Actually, no.  Unfortunately we still have some fire and forget APIs, so
> >the contention that we always have an open refcounted descriptor isn't
> >always true.

What API's, and what usage?

>  Yeap, you're right.  So, what we have is

>  * All high-level users have open access to the scsi high-level
>    object on issueing requests, but may close it before its requests
>    complete.

>  * All mid-layer users do get_device() before submitting requests,
>    but may put_device() before its requests complete.

Any LLDD's issuing requests should be doing a get/put around the request.

Any upper level drivers calling scsi_device_put() before a request
completes is likely a bug. sg has code in place to handle the
post-release/close completion of IO (IMO, a bad design).

Are any upper level drivers calling scsi_device_put() while they have
outstanding IO?

The scan code never calls upper level drivers probe functions via
device_add unless we are going to keep the scsi_device (well, there are
error paths in scsi_sysfs_add_sdev that look bad - we don't check the
result of scsi_sysfs_add_sdev). But for completeness, we could add
get/puts to the scan code.

As you pointed out, the current get_device() will never return NULL when
called via:

	get_device(&sdev->sdev_gendev)

The current code only narrows the window where problems might occur, I
don't see how it can completely avoid races with removal.

And the patch removes code from the mainline scsi IO paths.

-- Patrick Mansfield

  reply	other threads:[~2005-03-29 17:02 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-03-23  2:14 [PATCH scsi-misc-2.6 00/08] scsi: small fixes & cleanups Tejun Heo
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 01/08] scsi: remove unused bounce-buffer release path Tejun Heo
2005-03-23  4:07   ` James Bottomley
2005-03-23  6:08     ` Tejun Heo
2005-03-23 15:27       ` Jens Axboe
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 02/08] scsi: don't use blk_insert_request() for requeueing Tejun Heo
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 03/08] scsi: remove unused scsi_cmnd->internal_timeout field Tejun Heo
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 04/08] scsi: remove meaningless volatile qualifiers from structure definitions Tejun Heo
2005-03-23  4:15   ` James Bottomley
2005-03-23  4:22     ` Jeff Garzik
2005-03-23  5:28       ` Tejun Heo
2005-03-23 15:16       ` James Bottomley
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 05/08] scsi: remove a timer race from scsi_queue_insert() and cleanup timer Tejun Heo
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 06/08] scsi: remove meaningless scsi_cmnd->serial_number_at_timeout field Tejun Heo
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 07/08] scsi: remove bogus {get|put}_device() calls Tejun Heo
2005-03-23  4:15   ` James Bottomley
2005-03-23  9:13     ` Tejun Heo
2005-03-29 17:02       ` Patrick Mansfield [this message]
2005-03-23  2:14 ` [PATCH scsi-misc-2.6 08/08] scsi: fix hot unplug sequence Tejun Heo
2005-03-23  4:08   ` James Bottomley
2005-03-23  4:50     ` Tejun Heo
2005-03-23  7:19       ` Jens Axboe
2005-03-23 15:20         ` James Bottomley
2005-03-23 15:25           ` Jens Axboe
2005-03-25  0:45             ` James Bottomley
2005-03-25  3:15               ` Tejun Heo
2005-03-25  5:02                 ` James Bottomley
2005-03-25  5:38                   ` Tejun Heo
2005-03-25 19:19                     ` James Bottomley
2005-03-25 21:43                       ` Tejun Heo
2005-03-25 22:49                         ` James Bottomley
2005-03-26  7:27                       ` Kai Makisara
2005-03-26 14:48                         ` James Bottomley
2005-03-23 15:12       ` 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=20050329170201.GA6787@us.ibm.com \
    --to=patmans@us.ibm.com \
    --cc=James.Bottomley@SteelEye.com \
    --cc=axboe@suse.de \
    --cc=htejun@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /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.