From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Hannes Reinecke <hare@suse.de>,
Kay Sievers <kay.sievers@vrfy.org>,
SCSI development list <linux-scsi@vger.kernel.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Andrew Morton <akpm@linux-foundation.org>,
Greg Kroah-Hartman <gregkh@suse.de>,
Kernel development list <linux-kernel@vger.kernel.org>,
Tejun Heo <tj@kernel.org>,
Cornelia Huck <cornelia.huck@de.ibm.com>,
linux-fsdevel@vger.kernel.org,
"Eric W. Biederman" <ebiederm@aristanetworks.com>
Subject: Re: [PATCH 25/20] sysfs: Only support removing emtpy sysfs directories.
Date: Wed, 27 May 2009 21:42:30 +0000 [thread overview]
Message-ID: <1243460550.6067.82.camel@localhost.localdomain> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0905271710040.2669-100000@iolanthe.rowland.org>
On Wed, 2009-05-27 at 17:31 -0400, Alan Stern wrote:
> On Wed, 27 May 2009, James Bottomley wrote:
>
> > > I can't tell whether you understood my point. After a scsi_device is
> > > unregistered but before it is released -- i.e., when its state is
> > > SDEV_DEL -- it _is_ essentially unusable. So why wait until it is
> > > released to decrement the target's device counter? Why not do the
> > > decrement in __scsi_remove_device()?
> >
> > because the use model of the device still requires a valid target. Even
> > though it gets gated in several places in SDEV_DEL, we still have use of
> > the target parent. This is fixable, but only by a long audit of all the
> > sdev uses plus the enforcement of no use of target in DEL state rule,
> > which adds complexity.
>
> You're failing to distinguish properly between "delete" and "release".
> A target (or device in general) is deleted when it is removed from
> visibility -- i.e., when device_del() is called. It is released when
> the final put_device() call occurs and the data structure is
> deallocated.
I find the terms delete and release too close for comfort, which is why
I've always been careful to say remove from visibility.
> So, all I'm saying is there's nothing wrong with deleting a target
> when all its children are deleted, provided the target isn't released
> until all the children are released. Below you say the same thing.
>
>
> > > > Perhaps I haven't made the problem clear enough. You only want early
> > > > del if the host is going away, otherwise the target might be reused and
> > > > it can't be if you've called del on it. So there needs to be an
> > > > integration into the host lifecycle in some form.
> > >
> > > Yes, granted. That integration doesn't have to be complicated.
> > > Basically, you just decrement the counters in all the targets when
> > > setting a host's state to SHOST_DEL or SHOST_DEL_RECOVERY. At that
> >
> > And SHOST_CANCEL and SHOST_CANCEL_RECOVERY.
>
> If you prefer. I thought SHOST_DEL would be more appropriate because
> it occurs after scsi_forget_host() is called. All those transitions
> occur in scsi_remove_host(), anyway.
I mean in all four states.
> > > point there's no reason to keep an unpopulated target around, right?
> >
> > If the child list were empty, sure. However, it's likely not going to
> > be at this point.
>
> Regardless, it will work either way.
>
> > > Up until that point, the counter's value should be one more than the
> > > number of underlying sdevs. So if the counter decrements to 0 then
> > > there were no underlying sdevs and the target is deleted immediately;
> > > otherwise it is deleted when the last remaining sdev is deleted.
> >
> > No, that's the problem. It can be removed from visibility if it has no
> > visible sdevs, but it can't be deleted until the last sdev is released.
>
> Allow me to rephrase this: A target can be removed from visibility if
> it has no visible sdevs, but it can't be _released_ until the last sdev
> is released.
>
> That's fine. You remove a target from visibility when target->reap_ref
> becomes 0. The target isn't released until the target's embedded
> struct device's refcount becomes 0. To make this work, simply have
> scsi_alloc_sdev() call
>
> get_device(&starget->dev);
>
> and have scsi_device_dev_release_usercontext() call
>
> put_device(&starget->dev);
>
> Doesn't that do exactly what you're asking for?
That's um what we do to day ... the addition has to be to the visibility
management.
James
next prev parent reply other threads:[~2009-05-27 21:42 UTC|newest]
Thread overview: 200+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-20 4:09 [PATCH 0/20] Sysfs cleanups Eric W. Biederman
2009-05-20 15:37 ` Greg KH
2009-05-20 23:04 ` Eric W. Biederman
2009-05-21 0:27 ` [PATCH 01/20] sysfs: Implement sysfs_rename_link Eric W. Biederman
2009-05-21 0:27 ` [PATCH 02/20] driver core: Use sysfs_rename_link in device_rename Eric W. Biederman
2009-05-21 0:27 ` [PATCH 03/20] sysfs: Remove now unnecessary error reporting suppression Eric W. Biederman
2009-05-21 0:27 ` [PATCH 04/20] sysfs: Handle the general case of removing of directories with subdirectories Eric W. Biederman
2009-05-21 0:27 ` [PATCH 05/20] sysfs: Rename sysfs_d_iput to sysfs_dentry_iput Eric W. Biederman
2009-05-21 0:28 ` [PATCH 06/20] sysfs: Use dentry_ops instead of directly playing with the dcache Eric W. Biederman
2009-05-21 0:28 ` [PATCH 07/20] sysfs: Simplify sysfs_chmod_file semantics Eric W. Biederman
2009-05-21 0:28 ` [PATCH 08/20] sysfs: Optimize just changing the sysfs file mode Eric W. Biederman
2009-05-21 0:28 ` [PATCH 09/20] sysfs: Simplify iattr assignments Eric W. Biederman
2009-05-21 0:28 ` [PATCH 10/20] sysfs: Fix locking and factor out sysfs_sd_setattr Eric W. Biederman
2009-05-21 0:28 ` [PATCH 11/20] sysfs: Update s_iattr on link and unlink Eric W. Biederman
2009-05-21 0:28 ` [PATCH 12/20] sysfs: Nicely indent sysfs_symlink_inode_operations Eric W. Biederman
2009-05-21 0:28 ` [PATCH 13/20] sysfs: Implement sysfs_getattr & sysfs_permission Eric W. Biederman
2009-05-21 0:28 ` [PATCH 14/20] sysfs: In sysfs_chmod_file lazily propagate the mode change Eric W. Biederman
2009-05-21 0:28 ` [PATCH 15/20] sysfs: Kill sysfs_addrm_start and sysfs_addrm_finish Eric W. Biederman
2009-05-21 0:28 ` [PATCH 16/20] sysfs: Propagate renames to the vfs on demand Eric W. Biederman
2009-05-21 0:28 ` [PATCH 17/20] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Eric W. Biederman
2009-05-21 0:28 ` [PATCH 18/20] sysfs: Pass super_block to sysfs_get_inode Eric W. Biederman
2009-05-21 0:28 ` [PATCH 19/20] sysfs: Kill unused sysfs_sb variable Eric W. Biederman
2009-05-21 0:28 ` [PATCH 20/20] sysfs: Normalize error handling in sysfs_fill_inode Eric W. Biederman
2009-05-21 9:43 ` Tejun Heo
2009-05-21 10:29 ` Eric W. Biederman
2009-05-21 9:42 ` [PATCH 17/20] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Tejun Heo
2009-05-21 9:41 ` [PATCH 16/20] sysfs: Propagate renames to the vfs on demand Tejun Heo
2009-05-21 9:31 ` [PATCH 15/20] sysfs: Kill sysfs_addrm_start and sysfs_addrm_finish Tejun Heo
2009-05-21 9:16 ` [PATCH 14/20] sysfs: In sysfs_chmod_file lazily propagate the mode change Tejun Heo
2009-05-21 9:14 ` [PATCH 13/20] sysfs: Implement sysfs_getattr & sysfs_permission Tejun Heo
2009-05-21 7:42 ` [PATCH 12/20] sysfs: Nicely indent sysfs_symlink_inode_operations Tejun Heo
2009-05-21 8:42 ` [PATCH 11/20] sysfs: Update s_iattr on link and unlink Tejun Heo
2009-05-21 7:42 ` [PATCH 10/20] sysfs: Fix locking and factor out sysfs_sd_setattr Tejun Heo
2009-05-21 7:31 ` [PATCH 09/20] sysfs: Simplify iattr assignments Tejun Heo
2009-05-21 7:29 ` [PATCH 08/20] sysfs: Optimize just changing the sysfs file mode Tejun Heo
2009-05-21 7:54 ` Eric W. Biederman
2009-05-21 8:41 ` Tejun Heo
2009-05-21 6:42 ` [PATCH 07/20] sysfs: Simplify sysfs_chmod_file semantics Tejun Heo
2009-05-21 6:41 ` [PATCH 06/20] sysfs: Use dentry_ops instead of directly playing with the dcache Tejun Heo
2009-05-21 7:37 ` Eric W. Biederman
2009-05-21 7:40 ` Tejun Heo
2009-05-21 6:24 ` [PATCH 05/20] sysfs: Rename sysfs_d_iput to sysfs_dentry_iput Tejun Heo
2009-05-21 6:23 ` [PATCH 04/20] sysfs: Handle the general case of removing of directories with subdirectories Tejun Heo
2009-05-21 7:29 ` Eric W. Biederman
2009-05-21 7:36 ` Tejun Heo
2009-05-21 8:04 ` Eric W. Biederman
2009-05-21 8:37 ` Tejun Heo
2009-05-21 9:18 ` Eric W. Biederman
2009-05-21 9:28 ` Tejun Heo
2009-05-23 6:33 ` Eric W. Biederman
2009-05-23 11:35 ` Kay Sievers
2009-05-23 20:09 ` Eric W. Biederman
2009-05-23 20:46 ` Kay Sievers
2009-05-21 5:37 ` [PATCH 03/20] sysfs: Remove now unnecessary error reporting suppression Tejun Heo
2009-05-21 6:12 ` Eric W. Biederman
2009-05-21 6:20 ` Tejun Heo
2009-05-21 1:49 ` [PATCH 01/20] sysfs: Implement sysfs_rename_link Tejun Heo
2009-05-21 5:35 ` Tejun Heo
2009-05-21 10:06 ` Kay Sievers
2009-05-21 10:29 ` Eric W. Biederman
2009-05-21 11:40 ` Kay Sievers
2009-05-28 0:14 ` Greg KH
2009-05-28 0:30 ` Kay Sievers
2009-05-28 0:37 ` Greg KH
2009-05-28 22:58 ` [PATCH 0/24] sysfs cleanups Eric W. Biederman
2009-05-28 23:00 ` [PATCH 01/24] sysfs: Implement sysfs_rename_link Eric W. Biederman
2009-05-28 23:00 ` [PATCH 02/24] driver core: Use sysfs_rename_link in device_rename Eric W. Biederman
2009-05-28 23:00 ` [PATCH 03/24] sysfs: Remove now unnecessary error reporting suppression Eric W. Biederman
2009-05-28 23:00 ` [PATCH 04/24] sysfs: Normalize removing sysfs directories Eric W. Biederman
2009-05-29 9:14 ` Tejun Heo
2009-05-29 16:52 ` Eric W. Biederman
2009-05-30 10:43 ` Tejun Heo
2009-05-30 13:07 ` Eric W. Biederman
2009-05-30 13:20 ` Tejun Heo
2009-05-30 14:29 ` Eric W. Biederman
2009-05-30 13:59 ` Kay Sievers
2009-05-30 14:19 ` James Bottomley
2009-05-30 15:15 ` Eric W. Biederman
2009-05-30 15:51 ` James Bottomley
2009-05-30 21:20 ` Eric W. Biederman
2009-05-28 23:00 ` [PATCH 05/24] sysfs: Rename sysfs_d_iput to sysfs_dentry_iput Eric W. Biederman
2009-05-28 23:00 ` [PATCH 06/24] sysfs: Use dentry_ops instead of directly playing with the dcache Eric W. Biederman
2009-05-28 23:00 ` [PATCH 07/24] sysfs: Simplify sysfs_chmod_file semantics Eric W. Biederman
2009-05-28 23:00 ` [PATCH 08/24] sysfs: Optimize just changing the sysfs file mode Eric W. Biederman
2009-05-28 23:00 ` [PATCH 09/24] sysfs: Simplify iattr assignments Eric W. Biederman
2009-05-28 23:00 ` [PATCH 10/24] sysfs: Fix locking and factor out sysfs_sd_setattr Eric W. Biederman
2009-05-28 23:00 ` [PATCH 11/24] sysfs: Update s_iattr on link and unlink Eric W. Biederman
2009-05-28 23:00 ` [PATCH 12/24] sysfs: Nicely indent sysfs_symlink_inode_operations Eric W. Biederman
2009-05-28 23:00 ` [PATCH 13/24] sysfs: Implement sysfs_getattr & sysfs_permission Eric W. Biederman
2009-05-28 23:00 ` [PATCH 14/24] sysfs: In sysfs_chmod_file lazily propagate the mode change Eric W. Biederman
2009-05-28 23:00 ` [PATCH 15/24] sysfs: Kill sysfs_addrm_start and sysfs_addrm_finish Eric W. Biederman
2009-05-28 23:00 ` [PATCH 16/24] sysfs: Propagate renames to the vfs on demand Eric W. Biederman
2009-05-28 23:00 ` [PATCH 17/24] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Eric W. Biederman
2009-05-28 23:00 ` [PATCH 18/24] sysfs: Pass super_block to sysfs_get_inode Eric W. Biederman
2009-05-28 23:01 ` [PATCH 19/24] sysfs: Kill unused sysfs_sb variable Eric W. Biederman
2009-05-28 23:01 ` [PATCH 20/24] sysfs: Normalize error handling in sysfs_fill_inode Eric W. Biederman
2009-05-28 23:01 ` [PATCH 21/24] sysfs: Rename sysfs_mv_dir sysfs_rename Eric W. Biederman
2009-05-28 23:01 ` [PATCH 22/24] sysfs: Make sysfs_rename_link atomic Eric W. Biederman
2009-05-29 9:16 ` Tejun Heo
2009-05-29 17:17 ` Eric W. Biederman
2009-05-30 10:48 ` Tejun Heo
2009-05-28 23:01 ` [PATCH 23/24] driver core: Don't remove kobjects in device_shutdown Eric W. Biederman
2009-05-28 23:01 ` [PATCH 24/24] sysfs: In sysfs_add_one fail if the targe directory has been removed Eric W. Biederman
2009-05-29 9:18 ` Tejun Heo
2009-05-29 20:18 ` [PATCH 0/26] sysfs cleanups v3 Eric W. Biederman
2009-05-29 20:19 ` [PATCH 01/26] sysfs: Implement sysfs_rename_link Eric W. Biederman
2009-06-02 22:57 ` patch sysfs-implement-sysfs_rename_link.patch added to gregkh-2.6 tree gregkh
2009-05-29 20:19 ` [PATCH 02/26] driver core: Use sysfs_rename_link in device_rename Eric W. Biederman
2009-06-02 22:57 ` patch driver-core-use-sysfs_rename_link-in-device_rename.patch added to gregkh-2.6 tree gregkh
2009-05-29 20:19 ` [PATCH 03/26] sysfs: Remove now unnecessary error reporting suppression Eric W. Biederman
2009-06-02 22:57 ` patch sysfs-remove-now-unnecessary-error-reporting-suppression.patch added to gregkh-2.6 tree gregkh
2009-05-29 20:19 ` [PATCH 04/26] sysfs: sysfs_remove_dir stop checking for bogus cases Eric W. Biederman
2009-06-03 23:53 ` Greg KH
2009-06-04 0:41 ` Eric W. Biederman
2009-05-29 20:19 ` [PATCH 05/26] sysfs: Improve sysfs directory deletion debugging Eric W. Biederman
2009-05-29 20:19 ` [PATCH 06/26] sysfs: Don't hold addrm_start/addrm_finish over multiple removals Eric W. Biederman
2009-05-29 20:19 ` [PATCH 07/26] sysfs: Rename sysfs_d_iput to sysfs_dentry_iput Eric W. Biederman
2009-05-29 20:19 ` [PATCH 08/26] sysfs: Use dentry_ops instead of directly playing with the dcache Eric W. Biederman
2009-05-29 20:19 ` [PATCH 09/26] sysfs: Simplify sysfs_chmod_file semantics Eric W. Biederman
2009-05-29 20:19 ` [PATCH 10/26] sysfs: Optimize just changing the sysfs file mode Eric W. Biederman
2009-05-29 20:19 ` [PATCH 11/26] sysfs: Simplify iattr assignments Eric W. Biederman
2009-05-29 20:19 ` [PATCH 12/26] sysfs: Fix locking and factor out sysfs_sd_setattr Eric W. Biederman
2009-05-29 20:19 ` [PATCH 13/26] sysfs: Update s_iattr on link and unlink Eric W. Biederman
2009-05-29 20:19 ` [PATCH 14/26] sysfs: Nicely indent sysfs_symlink_inode_operations Eric W. Biederman
2009-05-29 20:19 ` [PATCH 15/26] sysfs: Implement sysfs_getattr & sysfs_permission Eric W. Biederman
2009-05-29 20:19 ` [PATCH 16/26] sysfs: In sysfs_chmod_file lazily propagate the mode change Eric W. Biederman
2009-05-29 20:19 ` [PATCH 17/26] sysfs: Kill sysfs_addrm_start and sysfs_addrm_finish Eric W. Biederman
2009-05-29 20:19 ` [PATCH 18/26] sysfs: Propagate renames to the vfs on demand Eric W. Biederman
2009-05-29 20:19 ` [PATCH 19/26] sysfs: Merge sysfs_rename_dir and sysfs_move_dir Eric W. Biederman
2009-05-29 20:19 ` [PATCH 20/26] sysfs: Pass super_block to sysfs_get_inode Eric W. Biederman
2009-05-29 20:19 ` [PATCH 21/26] sysfs: Kill unused sysfs_sb variable Eric W. Biederman
2009-05-29 20:19 ` [PATCH 22/26] sysfs: Normalize error handling in sysfs_fill_inode Eric W. Biederman
2009-05-29 20:19 ` [PATCH 23/26] sysfs: Rename sysfs_mv_dir sysfs_rename Eric W. Biederman
2009-05-29 20:19 ` [PATCH 24/26] sysfs: Make sysfs_rename_link atomic Eric W. Biederman
2009-05-29 20:19 ` [PATCH 25/26] driver core: Don't remove kobjects in device_shutdown Eric W. Biederman
2009-05-29 20:19 ` [PATCH 26/26] sysfs: In sysfs_add_one fail if the targe directory has been removed Eric W. Biederman
2009-05-28 1:51 ` [PATCH 01/20] sysfs: Implement sysfs_rename_link Eric W. Biederman
2009-05-23 20:13 ` [PATCH 21/20] sysfs: Rename sysfs_mv_dir sysfs_rename Eric W. Biederman
2009-05-23 20:13 ` [PATCH 22/20] sysfs: Make sysfs_rename_link atomic Eric W. Biederman
2009-05-23 21:32 ` Kay Sievers
2009-05-23 23:21 ` Kay Sievers
2009-05-24 13:03 ` Kay Sievers
2009-05-23 20:13 ` [PATCH 23/20] driver core: Don't remove kobjects in device_shutdown Eric W. Biederman
2009-05-23 22:15 ` Kay Sievers
2009-05-23 20:13 ` [PATCH 24/20] sysfs: In sysfs_add_one fail if the targe directory has been removed Eric W. Biederman
2009-05-23 21:29 ` Kay Sievers
2009-05-23 20:13 ` [PATCH 25/20] sysfs: Only support removing emtpy sysfs directories Eric W. Biederman
2009-05-23 21:27 ` Kay Sievers
2009-05-24 12:59 ` Kay Sievers
2009-05-24 14:17 ` Eric W. Biederman
2009-05-24 15:20 ` Kay Sievers
2009-05-25 2:06 ` Alan Stern
2009-05-25 11:45 ` Kay Sievers
2009-05-25 12:01 ` Kay Sievers
2009-05-25 15:49 ` Alan Stern
2009-05-25 18:19 ` Kay Sievers
2009-05-25 20:14 ` Alan Stern
2009-05-26 16:27 ` Kay Sievers
2009-05-26 19:29 ` Alan Stern
2009-05-26 21:09 ` James Bottomley
2009-05-26 21:13 ` Kay Sievers
2009-05-26 21:56 ` Alan Stern
2009-05-26 22:03 ` Kay Sievers
2009-05-26 23:49 ` James Bottomley
2009-05-27 0:02 ` Kay Sievers
2009-05-27 2:17 ` Alan Stern
2009-05-27 11:35 ` Hannes Reinecke
2009-05-27 16:01 ` James Bottomley
2009-05-27 16:16 ` Alan Stern
2009-05-27 16:24 ` James Bottomley
2009-05-27 17:01 ` Alan Stern
2009-05-27 17:08 ` James Bottomley
2009-05-27 18:07 ` Alan Stern
2009-05-27 19:44 ` James Bottomley
2009-05-27 20:40 ` Alan Stern
2009-05-27 20:49 ` James Bottomley
2009-05-27 21:31 ` Alan Stern
2009-05-27 21:42 ` James Bottomley [this message]
2009-05-27 22:15 ` Alan Stern
2009-05-27 22:22 ` James Bottomley
2009-05-28 15:24 ` Alan Stern
2009-05-28 15:45 ` Eric W. Biederman
2009-05-28 17:51 ` Alan Stern
2009-05-28 18:21 ` James Bottomley
2009-05-28 20:02 ` Alan Stern
2009-05-28 20:10 ` James Bottomley
2009-05-28 21:04 ` Alan Stern
2009-05-29 12:32 ` Hannes Reinecke
2009-05-29 20:08 ` Alan Stern
2009-05-27 18:00 ` Eric W. Biederman
2009-05-27 18:15 ` Alan Stern
2009-05-27 18:24 ` Eric W. Biederman
2009-05-27 21:38 ` Alan Stern
2009-05-27 22:06 ` Eric W. Biederman
2009-05-27 22:18 ` Alan Stern
2009-05-26 21:39 ` Alan Stern
2009-05-25 7:44 ` Eric W. Biederman
2009-05-25 7:53 ` Eric W. Biederman
2009-05-25 10:51 ` Kay Sievers
2009-05-24 3:24 ` Tejun Heo
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=1243460550.6067.82.camel@localhost.localdomain \
--to=james.bottomley@hansenpartnership.com \
--cc=akpm@linux-foundation.org \
--cc=cornelia.huck@de.ibm.com \
--cc=ebiederm@aristanetworks.com \
--cc=ebiederm@xmission.com \
--cc=gregkh@suse.de \
--cc=hare@suse.de \
--cc=kay.sievers@vrfy.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
--cc=tj@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 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).