linux-kernel - Re: [PATCH 5/5] blkcg: remove blkio_group->stats

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] [day] [month] [year] [list]

Message-ID: <20120309180312.GC24890@google.com>
Date:	Fri, 9 Mar 2012 10:03:12 -0800
From:	Tejun Heo <tj@...nel.org>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	axboe@...nel.dk, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, dpshah@...gle.com,
	ctalbott@...gle.com, rni@...gle.com
Subject: Re: [PATCH 5/5] blkcg: remove blkio_group->stats_lock

Hey, Vivek.

On Fri, Mar 09, 2012 at 12:15:23PM -0500, Vivek Goyal wrote:
> On Thu, Mar 08, 2012 at 10:54:00AM -0800, Tejun Heo wrote:
> > With recent plug merge updates, all non-percpu stat updates happen
> > under queue_lock making stats_lock unnecessary to synchronize stat
> > updates.  The only synchronization necessary is stat reading, which
> > can be done using u64_stats_sync instead.
> > 
> > This patch removes blkio_group->stats_lock and adds
> > blkio_group_stats->syncp for reader synchronization.
> 
> Good to see stat_lock going away. That lock was confusing as we were doing
> updates under queue_lock anyway. One less lock to think about now.
> 
> This stat code is messy though. All time related stat, maintaining flags,
> dividing stats between read/write, sync/async. I think we are maintaining
> way too much of stat. (/me wished there was a way to get rid of some of
> the stats and make code simpler).

The biggest problem I'm having with the stat code is that it almost
mandates spaghettiness.  This stems both from the limitations of
cgroup files and the implementation.  blkcg needs to declare all
possible files upfront on creating cgroup directories, so it needs to
know all files which may be used by any policy implementation.  This
leads another layer of indirection in blkcg core and when a policy
wants to create a new file (including stat), it has to add it to the
blkcg core and then walk back.  This is inherently nasty.  I'm trying
to see whether cgroup can be updated to allow dynamic file
creation/removal so that policies can manage their own files instead
of having to dump everything into blkcg core.

Thanks.

-- 
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/