linux-kernel - Re: [PATCH 02/10] block: fix genhd refcounting in blkio_policy_parse_and

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20111020161123.GW25124@google.com>
Date:	Thu, 20 Oct 2011 09:11:23 -0700
From:	Tejun Heo <tj@...nel.org>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org, ctalbott@...gle.com,
	rni@...gle.com, containers@...ts.linux-foundation.org,
	kay.sievers@...y.org
Subject: Re: [PATCH 02/10] block: fix genhd refcounting in
 blkio_policy_parse_and_set()

Hello,

On Thu, Oct 20, 2011 at 09:41:37AM -0400, Vivek Goyal wrote:
> > Moreover, it doesn't even make the implementation simpler.  blkiocg
> > currently keeps a separate list of policies so that they don't
> > disappear along with blkg's.
> 
> One reason for keeping rules in blkiocg is that blkg don't get created
> until and unless IO happens in that cgroup. Rules can be created much
> before that.

Yeah, I'd like to change that.  I suppose it's result of evolution but
it's not like rule addition is hot path or currently doesn't lookup
bdev anyway.

> > The only way applying rules to dynamic devices can work is doing the
> > proper dynamic configuration off udev and friends.
> 
> Actually it is not exactly a feature at this point of time. It was just
> for the sake of simplicity that I let the rules be there even if device
> has gone away and yes it is indeep a shortcoming that if a different
> device shows up with old device's major and minor, then old rule will
> get applied to new device.
> 
> Having said that, removal of rule upon device removal also might not
> make much sense.
> 
> - Rules are tied to cgroups and not to devices as such. So until cgroup
>   goes away a user might be surprised that a configured rule for a device
>   suddenly disappeared.

Rules are tied to their group-device pair and removal of either part
should remove the rule.  I mean, you're looking up and rejecting
creation of new rules if the device isn't there.

> - Though my examples are not exactly similar, but when a device goes away
>   we don't try to unmount the filesystem automatically. We don't try to
>   get rid of /etc/fstab entries and if somebody as put a /etc/fstab entry
>   based on device name, then they might end up mounting wrong device.

That is different because the device node the filesystem holds onto is
decommissioned and essentially put into zombie state for residual
reference draining.  It is NEVER re-used for any other purpose.  If we
need that type of ref draining, sure, we can do it, but there is no
reason to do that for rules at all.

> So I don't feel strongly to tie rules and device life time together.
> Making use of udev and friends to automatically add/remove rules as
> devices show up or go will make sense though.

I think this is essentially a bug.  If you have something like "dev =
find_and_get(); put(dev); return dev != NULL;", it's a pretty good
indication something is pretty screwed there, so unless someone
screams really loud, I think I'm gonna push for removal of the
feature.

Thank you.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/