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

2013-05-22 Thread Tejun Heo
Hey, On Wed, May 22, 2013 at 02:50:14PM -0400, Mike Snitzer wrote: > Was wondering: how is percpu-refcounting coming along? Do you have a > pointer to the code that can be pulled in for use by Mikulas' dm-crypt > changes? > > Would be nice to get this stuff sorted out for the 3.11 merge window.

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

2013-05-22 Thread Mike Snitzer
On Thu, Apr 18 2013 at 1:03pm -0400, Tejun Heo wrote: > Hello, Mike. > > On Thu, Apr 18, 2013 at 12:47:42PM -0400, Mike Snitzer wrote: > > I see you nack and raise you with: please reconsider in the near term. > > The thing is that percpu-refcnting is already in mostly ready-form, so > unless

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

2013-05-22 Thread Mike Snitzer
On Thu, Apr 18 2013 at 1:03pm -0400, Tejun Heo t...@kernel.org wrote: Hello, Mike. On Thu, Apr 18, 2013 at 12:47:42PM -0400, Mike Snitzer wrote: I see you nack and raise you with: please reconsider in the near term. The thing is that percpu-refcnting is already in mostly ready-form, so

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

2013-05-22 Thread Tejun Heo
Hey, On Wed, May 22, 2013 at 02:50:14PM -0400, Mike Snitzer wrote: Was wondering: how is percpu-refcounting coming along? Do you have a pointer to the code that can be pulled in for use by Mikulas' dm-crypt changes? Would be nice to get this stuff sorted out for the 3.11 merge window.

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

2013-04-18 Thread Tejun Heo
Hello, Mike. On Thu, Apr 18, 2013 at 12:47:42PM -0400, Mike Snitzer wrote: > I see you nack and raise you with: please reconsider in the near term. The thing is that percpu-refcnting is already in mostly ready-form, so unless this dm series is planned to be merged for v3.10-rc1, I don't see the

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

2013-04-18 Thread Mike Snitzer
On Tue, Apr 16 2013 at 1:24pm -0400, Tejun Heo wrote: > 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 > >

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

2013-04-18 Thread Mike Snitzer
On Tue, Apr 16 2013 at 1:24pm -0400, Tejun Heo t...@kernel.org wrote: 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

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

2013-04-18 Thread Tejun Heo
Hello, Mike. On Thu, Apr 18, 2013 at 12:47:42PM -0400, Mike Snitzer wrote: I see you nack and raise you with: please reconsider in the near term. The thing is that percpu-refcnting is already in mostly ready-form, so unless this dm series is planned to be merged for v3.10-rc1, I don't see the

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

2013-04-16 Thread Mikulas Patocka
On Tue, 16 Apr 2013, Tejun Heo wrote: > 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

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

2013-04-16 Thread Tejun Heo
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

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

2013-04-16 Thread Tejun Heo
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

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

2013-04-16 Thread Mikulas Patocka
On Tue, 16 Apr 2013, Tejun Heo wrote: 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

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

2013-04-15 Thread Mikulas Patocka
On Fri, 12 Apr 2013, Tejun Heo wrote: > On Fri, Apr 12, 2013 at 02:01:08PM -0400, Mikulas Patocka wrote: > > So if you think that reference counts should be incremented by every clone > > of the original bio, what kind of bug should it protect against? If we > > don't increment reference

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

2013-04-15 Thread Mikulas Patocka
On Fri, 12 Apr 2013, Tejun Heo wrote: On Fri, Apr 12, 2013 at 02:01:08PM -0400, Mikulas Patocka wrote: So if you think that reference counts should be incremented by every clone of the original bio, what kind of bug should it protect against? If we don't increment reference counts for

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

2013-04-12 Thread Tejun Heo
On Fri, Apr 12, 2013 at 02:01:08PM -0400, Mikulas Patocka wrote: > So if you think that reference counts should be incremented by every clone > of the original bio, what kind of bug should it protect against? If we > don't increment reference counts for pages, why should we do it for cgroup >

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

2013-04-12 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: > On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: > > All that I can tell you is that adding an empty atomic operation > > "cmpxchg(>bi_css->refcnt, bio->bi_css->refcnt, bio->bi_css->refcnt);" > > to bio_clone_context and

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

2013-04-12 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: > On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: > > If this becomes an actual bottleneck, the right thing to do is making > > css ref per-cpu. Please stop messing around with refcounting. > > If you think this kind of hackery is acceptable,

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

2013-04-12 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: If this becomes an actual bottleneck, the right thing to do is making css ref per-cpu. Please stop messing around with refcounting. If you think this kind of hackery is acceptable, you

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

2013-04-12 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: All that I can tell you is that adding an empty atomic operation cmpxchg(bio-bi_css-refcnt, bio-bi_css-refcnt, bio-bi_css-refcnt); to bio_clone_context and bio_disassociate_task

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

2013-04-12 Thread Tejun Heo
On Fri, Apr 12, 2013 at 02:01:08PM -0400, Mikulas Patocka wrote: So if you think that reference counts should be incremented by every clone of the original bio, what kind of bug should it protect against? If we don't increment reference counts for pages, why should we do it for cgroup

Re: [PATCH v2] make dm and dm-crypt forward cgroup context

2013-04-11 Thread Milan Broz
On 12.4.2013 2:22, Tejun Heo wrote: > On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: >> All that I can tell you is that adding an empty atomic operation >> "cmpxchg(>bi_css->refcnt, bio->bi_css->refcnt, bio->bi_css->refcnt);" >> to bio_clone_context and bio_disassociate_task

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: > All that I can tell you is that adding an empty atomic operation > "cmpxchg(>bi_css->refcnt, bio->bi_css->refcnt, bio->bi_css->refcnt);" > to bio_clone_context and bio_disassociate_task increases the time to run a > benchmark

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

2013-04-11 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: > On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: > > If this becomes an actual bottleneck, the right thing to do is making > > css ref per-cpu. Please stop messing around with refcounting. > > If you think this kind of hackery is acceptable,

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: > If this becomes an actual bottleneck, the right thing to do is making > css ref per-cpu. Please stop messing around with refcounting. If you think this kind of hackery is acceptable, you really need to re-evaluate your priorities in

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 03:49:20PM -0400, Mikulas Patocka wrote: > If the bi_css pointer points to a structure that is shared between > processes, using atomic instruction causes cache line boucing - it doesn't > cost a few instructions, it costs 2-3 hundreds cycles. > > I modified the patch to

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

2013-04-11 Thread Mikulas Patocka
On Wed, 10 Apr 2013, Tejun Heo wrote: > On Wed, Apr 10, 2013 at 07:42:59PM -0400, Mikulas Patocka wrote: > > /* > > + * bio_clone_context copies cgroup context from the original bio to the > > new bio. > > + * It is used by bio midlayer drivers that create new bio based on an > > original >

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

2013-04-11 Thread Mikulas Patocka
On Wed, 10 Apr 2013, Tejun Heo wrote: On Wed, Apr 10, 2013 at 07:42:59PM -0400, Mikulas Patocka wrote: /* + * bio_clone_context copies cgroup context from the original bio to the new bio. + * It is used by bio midlayer drivers that create new bio based on an original + * bio and

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 03:49:20PM -0400, Mikulas Patocka wrote: If the bi_css pointer points to a structure that is shared between processes, using atomic instruction causes cache line boucing - it doesn't cost a few instructions, it costs 2-3 hundreds cycles. I modified the patch to use

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: If this becomes an actual bottleneck, the right thing to do is making css ref per-cpu. Please stop messing around with refcounting. If you think this kind of hackery is acceptable, you really need to re-evaluate your priorities in

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

2013-04-11 Thread Mikulas Patocka
On Thu, 11 Apr 2013, Tejun Heo wrote: On Thu, Apr 11, 2013 at 12:52:03PM -0700, Tejun Heo wrote: If this becomes an actual bottleneck, the right thing to do is making css ref per-cpu. Please stop messing around with refcounting. If you think this kind of hackery is acceptable, you

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

2013-04-11 Thread Tejun Heo
On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: All that I can tell you is that adding an empty atomic operation cmpxchg(bio-bi_css-refcnt, bio-bi_css-refcnt, bio-bi_css-refcnt); to bio_clone_context and bio_disassociate_task increases the time to run a benchmark from 23 to

Re: [PATCH v2] make dm and dm-crypt forward cgroup context

2013-04-11 Thread Milan Broz
On 12.4.2013 2:22, Tejun Heo wrote: On Thu, Apr 11, 2013 at 08:06:10PM -0400, Mikulas Patocka wrote: All that I can tell you is that adding an empty atomic operation cmpxchg(bio-bi_css-refcnt, bio-bi_css-refcnt, bio-bi_css-refcnt); to bio_clone_context and bio_disassociate_task increases the