linux-raid.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Tejun Heo <teheo@suse.de>
Cc: Neil Brown <neilb@suse.de>,
	linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
	Doug Ledford <dledford@redhat.com>, Greg KH <greg@kroah.com>,
	Jens Axboe <jens.axboe@oracle.com>
Subject: Re: [PATCH 1/2] md: make devices disappear when they are no longer needed.
Date: Mon, 24 Nov 2008 16:42:45 +0000	[thread overview]
Message-ID: <20081124164245.GD28946@ZenIV.linux.org.uk> (raw)
In-Reply-To: <492AD169.9000805@suse.de>

On Tue, Nov 25, 2008 at 01:08:09AM +0900, Tejun Heo wrote:

> Devices can be killed from userland via sysfs for SCSI or mdadm for
> md.  It's true that such approach is less convenient for unloading but
> if it can make usual cases easier, why not?

_What_ usual cases?

> > And the right way to deal with that is to have explicit boundaries for
> > "opened or in process of being opened"; we almost have them (probe and
> > final release), so the only point we are missing is on failure exit from
> > __blkdev_get()...
> > 
> > I really think that it's much saner than trying to change the lifetime
> > rules for gendisk, etc.
> 
> Well, I don't know.  It seems like a lot of trouble just to allow
> "rmmod something" without first killing the devices and as people are
> now so used to reference counted objects and ->release, not having it
> on cdev or gendisk is quite a PITA.  (BTW, Greg, can you please drop
> cdev->release patch for now, it's wrong as it currently stands).

gendisks *ARE* reference counted, damnit.  So are net_device and a lot
of other things.  And no, it's not true that "struct net_device exists"
implies "the low-level objects that once might have been related to it
still exist" either.

> Can you see any problem with caching ->disk_release existence on
> registration and wrap __module_get/put() around its invocation?  It
> wouldn't change behavior of any existing drivers and md can use it if
> it wants.  Doing "mdadm --stop --scan" would be enough to unload the
> module and md can do whatever forward or back reference it wants to do
> to work out the weird userland interface.

Other than general ugliness and special-casing where none is really needed?
Special-casing as "very different life cycle if special method is present"...

If anything, we need to go in opposite direction - give the drivers a way
to say "my underlying object is gone, STFU and don't bother me with that
gendisk ever again; free it when you are done with it, but from now on
any access to it would better fail.  Oh, and I might find a new device
in place of that any time now, so new open() would better get not fail
just something in VFS still has a reference to that gendisk".

Which is doable - note that we can unhash block_device, dissociate inodes
from it and let new open() DTRT.  Earlier opened files will still have
a reference to address_space of original block_device (which is why we
have file->f_mapping instead of going through ->f_dentry->d_inode->i_mapping),
so we are fine.

  reply	other threads:[~2008-11-24 16:42 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-24  3:55 [PATCH 0/2] RFC: allow md devices to disappear when not in use NeilBrown
2008-11-24  3:55 ` [PATCH 1/2] md: make devices disappear when they are no longer needed NeilBrown
2008-11-24  4:18   ` Tejun Heo
2008-11-24  5:13     ` Neil Brown
2008-11-24  5:34       ` Tejun Heo
2008-11-24  6:10         ` NeilBrown
2008-11-24  6:12           ` Tejun Heo
2008-11-24  6:24         ` Al Viro
2008-11-24  6:56           ` Tejun Heo
2008-11-24 13:31             ` Al Viro
2008-11-24 14:04               ` Tejun Heo
2008-11-24 14:26                 ` Tejun Heo
2008-11-24 14:48                   ` Al Viro
2008-11-24 16:08                     ` Tejun Heo
2008-11-24 16:42                       ` Al Viro [this message]
2008-11-24 17:18                         ` Tejun Heo
2008-11-28  0:23                     ` Neil Brown
2008-11-24  4:24   ` Al Viro
2008-11-24  4:47     ` Neil Brown
2008-11-24  6:38       ` Al Viro
2008-11-24  3:55 ` [PATCH 2/2] Allow md devices to be created by name NeilBrown

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=20081124164245.GD28946@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dledford@redhat.com \
    --cc=greg@kroah.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=teheo@suse.de \
    /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).