Re: [PATCH v2] make dm and dm-crypt forward cgroup context (was: dm-crypt parallelization patches)

[Tejun Heo <tj@kernel.org>]

Hey,

On Mon, Apr 15, 2013 at 09:02:06AM -0400, Mikulas Patocka wrote:
> The patch is not bug-prone, because we already must make sure that the 
> cloned bio has shorter lifetime than the master bio - so the patch doesn't 
> introduce any new possibilities to make bugs.

The whole world isn't composed of only your code.  As I said
repeatedly, you're introducing an API which is misleading and can
easily cause subtle bugs which are very difficult to reproduce.

Imagine it being used to tag a metatdata or checksum update bio being
sent down while processing another bio and used to "clone" the context
of the original bio.  It'll work most of the time even if the original
bio gets completed first but it'll break when it gets really unlucky -
e.g. racing with other operations which can put the base css ref, and
it'll be hellish to reproduce and everyone would have to pay for your
silly hack.

> > Do the two really look the same to you?  The page refs are much more
> > expensive, mostly contained in and the main focus of dm.  ioc/css refs
> > aren't that expensive to begin with, css refcnting is widely scattered
> 
> ioc is per-task, so it is likely to be cached (but there are processors 
> that have slow atomic operations even on cached data - on Pentium 4 it 
> takes about 100 cycles). But css is shared between tasks and produces the 
> cache ping-pong effect.

For $DIETY's sake, how many times do I have to tell you to use per-cpu
reference count?  Why do I have to repeat the same story over and over
again?  What part of "make the reference count per-cpu" don't you get?
It's not a complicated message.

At this point, I can't even understand why or what the hell you're
arguing.  There's a clearly better way to do it and you're just
repeating yourself like a broken record that your hack in itself isn't
broken.

So, if you wanna continue that way for whatever reason, you have my
firm nack and I'm outta this thread.

Bye bye.

-- 
tejun