dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Tejun Heo <tj@kernel.org>, Jens Axboe <axboe@kernel.dk>,
	Mike Snitzer <snitzer@redhat.com>,
	Milan Broz <gmazyland@gmail.com>,
	dm-devel@redhat.com, Andi Kleen <andi@firstfloor.org>,
	dm-crypt@saout.de, linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	Christian Schmidt <schmidt@digadd.de>
Subject: Re: dm-crypt parallelization patches
Date: Wed, 10 Apr 2013 15:24:27 -0400	[thread overview]
Message-ID: <20130410192427.GA14911@redhat.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1304091710150.24735@file.rdu.redhat.com>

On Tue, Apr 09, 2013 at 05:18:25PM -0400, Mikulas Patocka wrote:

[..]
> > bio_associate_current() return -EBUSY if bio has already been associated
> > with an io context.
> > 
> > So in a stack if every driver calls bio_associate_current(), upon bio
> > submission, it will automatically amke sure bio gets associated with
> > submitter task in top level device and calls by lower level device
> > will be ignored.
> 
> The stacking drivers do not pass the same bio to each other.
> 
> The stacking driver receives a bio, allocates zero or more new bios and 
> sends these new bios to the lower layer. So you need to propagate 
> ownership from the received bio to newly allocated bios, you don't want to 
> associate newly allocated bio with "current" process.

Actually I was asking to call bio_associate_current() for the incoming
bio in the driver and not for the newly created bios by the driver.

For any newly created bios on behalf of this incoming bio, we need to
copy the context so that this context info can be propogated down the
stack.


> 
> > Lower level devices I think just need to make sure this context
> > info is propogated to cloned bios.
> > 
> > 
> > [..]
> > > +#ifdef CONFIG_BLK_CGROUP
> > > +	tio->clone.bi_ioc = ci->bio->bi_ioc;
> > > +	tio->clone.bi_css = ci->bio->bi_css;
> > 
> > You also need to take references to ioc and css objects. I guess a helper
> > function will be better. May be something like.
> 
> The lifetime of the "tio" structure is shorter than the lifetime of 
> "ci->bio". So we don't need to increment reference.
> 
> We only need to increment reference if we copy ownership to a new bio that 
> has could have longer lifetime than the original bio. But this situation 
> is very rare - in most stacking drivers the newly allocated bio has 
> shorter lifetime than the original one.

I think it is not a good idea to rely on the fact that cloned bios or
newly created bios will have shorter lifetime than the original bio. In fact
when the bio completes and you free it up bio_disassociate_task() will try
to put io context and blkcg reference. So it is important to take
reference if you are copying context info in any newly created bio.

Thanks
Vivek

  reply	other threads:[~2013-04-10 19:24 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-26  3:47 dm-crypt performance Mikulas Patocka
2013-03-26  6:52 ` Christoph Hellwig
2013-03-26 12:27 ` [dm-devel] " Alasdair G Kergon
2013-03-26 20:05   ` Milan Broz
2013-03-26 20:28     ` Mike Snitzer
2013-03-26 20:58       ` Milan Broz
2013-03-28 18:53       ` Tejun Heo
2013-03-28 19:33         ` Vivek Goyal
2013-03-28 19:44           ` Tejun Heo
2013-03-28 20:38             ` Vivek Goyal
2013-03-28 20:45               ` Tejun Heo
2013-04-09 17:51                 ` dm-crypt parallelization patches Mikulas Patocka
2013-04-09 17:57                   ` Tejun Heo
2013-04-09 18:08                     ` Mikulas Patocka
2013-04-09 18:10                       ` Tejun Heo
2013-04-09 18:42                         ` Vivek Goyal
2013-04-09 18:57                           ` Tejun Heo
2013-04-09 19:13                             ` Vivek Goyal
2013-04-09 19:42                         ` Mikulas Patocka
2013-04-09 19:52                           ` Tejun Heo
2013-04-09 20:32                             ` Mikulas Patocka
2013-04-09 21:02                               ` Tejun Heo
2013-04-09 21:03                                 ` Tejun Heo
2013-04-09 21:07                               ` Vivek Goyal
2013-04-09 21:18                                 ` Mikulas Patocka
2013-04-10 19:24                                   ` Vivek Goyal [this message]
2013-04-10 23:42                                     ` [PATCH] make dm and dm-crypt forward cgroup context (was: dm-crypt parallelization patches) Mikulas Patocka
2013-04-10 23:50                                       ` Tejun Heo
2013-04-11 19:49                                         ` [PATCH v2] " Mikulas Patocka
2013-04-11 19:52                                           ` Tejun Heo
2013-04-11 20:00                                             ` Tejun Heo
2013-04-12  0:06                                               ` Mikulas Patocka
2013-04-12  0:22                                                 ` Tejun Heo
2013-04-12  5:59                                                   ` [PATCH v2] make dm and dm-crypt forward cgroup context Milan Broz
2013-04-12 18:17                                                   ` [PATCH v2] make dm and dm-crypt forward cgroup context (was: dm-crypt parallelization patches) Mikulas Patocka
2013-04-12 18:01                                               ` Mikulas Patocka
2013-04-12 18:29                                                 ` Tejun Heo
2013-04-15 13:02                                                   ` Mikulas Patocka
2013-04-16 17:24                                                     ` Tejun Heo
2013-04-16 19:41                                                       ` Mikulas Patocka
2013-04-18 16:47                                                       ` Mike Snitzer
2013-04-18 17:03                                                         ` Tejun Heo
2013-05-22 18:50                                                           ` Mike Snitzer
2013-05-22 19:48                                                             ` Tejun Heo
2013-04-09 18:36                   ` dm-crypt parallelization patches Vivek Goyal
     [not found]     ` <5151FF82.6090405-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-04-09 18:08       ` [dm-devel] dm-crypt performance Mikulas Patocka
2013-04-09 18:59         ` [dm-crypt] " Milan Broz
  -- strict thread matches above, loose matches on Subject: below --
2013-09-03 19:06 dm-crypt parallelization patches Mikulas Patocka
2013-09-03 20:07 ` Andi Kleen
2013-09-11 23:03   ` Mikulas Patocka
2013-09-11 23:33     ` Andi Kleen
2013-09-12  3:51     ` Milan Broz
2013-09-03 20:49 ` Milan Broz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130410192427.GA14911@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=axboe@kernel.dk \
    --cc=dm-crypt@saout.de \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=schmidt@digadd.de \
    --cc=snitzer@redhat.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).