All of lore.kernel.org
 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: 22+ 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: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 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.