Re: [BUG 2.6.17-git] kmem_cache_create: duplicate cache scsi_cmd_cache

From: Al Viro
Date: Fri May 12 2006 - 20:20:32 EST


On Fri, May 12, 2006 at 04:50:32PM -0700, Linus Torvalds wrote:
>
>
> On Sat, 13 May 2006, Al Viro wrote:
> >
> > BTW, the best option is to kill bdev_uevent() again. Short of that,
> > skip PHYSDEV mess if disk doesn't have GENHD_FL_UP.
>
> I do think the mount/umount events are valid and interesting, so I'd much
> rather see the second version.

mount/umount events are BS, but that's a separate story. In case you've
missed the original discussion:
* mount/umount is not tied to block device; if you really want
to track mount tree changes, you need much more; if you want to track
the set of filesystems being active, you _still_ need more.
* "block device is used by fs" is valid, but bdev_uevent() is _NOT_
that; we miss e.g. journals.
* "block device is being claimed exclusively" - even more interesting,
and again, bdev_uevent() isn't it.
* "underlying object of _the_ block device used by a filesystem"
is not well-defined, to put it mildly.

IOW, it's a big mess. Note that if you care about real mount events
("set of mounts has changed") you can do that already, and get _all_
of them - including mount --move, etc. poll(2) on /proc/mounts
will do that for you. And you don't need to be priveleged for that,
while we are at it.

That's what hald wanted bdev_uevent() for. All attempts to find what
other users are really trying to get had not brought anything.

I'm not saying that we don't need something similar, but let's try to make
sure it makes _sense_ and isn't just "oh, my userland code rereads too
often; guess if I would see when kernel reaches this, this and that point
I would be able to reduce the frequency of rereads and wouldn't miss too
much in the cases I've ran into so far".

In the current form bdev_uevent() makes no sense. It pushes a lot of
vaguely related stuff and doesn't cover _any_ sane use fully.

So yeah, I'm seriously pissed off at the bdev_uevent() merge - at hald
crowd for pushing it to gregkh, at gregkh for blindly passing it on
and at myself for missing it back then. Note that as soon as hald folks
_did_ explain what they really wanted, they've got a sane solution (and
are using it now just fine).

> However, that does beg the question: wouldn't that effectively be what the
> patch I posted would do? Notably the "disk->driverfs_dev = NULL" part
> after we've dropped it (the "KOBJ_REMOVE" event move is a separate issue,
> mixed here into the same patch, but should result in possibly better name
> generation for the event).
>
> Basically, onces driverfs_dev has been dropped, we NULL it out, and then
> the people who use it automatically get the right result.

More or less. We don't want to mess with refcounts at all; aside of that,
yes, checking if it's NULL is OK. subsystem mutex should be enough, AFAICS.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/