From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Cc: axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org,
yehuda-L5o5AL9CYN0tUFlbccrkMA@public.gmane.org,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
neilb-l3A5Bk7waGM@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
mpatocka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org,
agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
drbd-dev-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org
Subject: Re: [PATCH v2 01/14] block: Generalized bio pool freeing
Date: Thu, 24 May 2012 09:19:14 -0700 [thread overview]
Message-ID: <20120524161914.GD27983@google.com> (raw)
In-Reply-To: <20120524160944.GB27983-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On Thu, May 24, 2012 at 09:09:44AM -0700, Tejun Heo wrote:
> On Wed, May 23, 2012 at 05:02:38PM -0700, Kent Overstreet wrote:
> > With the old code, when you allocate a bio from a bio pool you have to
> > implement your own destructor that knows how to find the bio pool the
> > bio was originally allocated from.
> >
> > This adds a new field to struct bio (bi_pool) and changes
> > bio_alloc_bioset() to use it. This makes various bio destructors
> > unnecessary, so they're then deleted.
> >
> > Signed-off-by: Kent Overstreet <koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > Change-Id: I5eb66c1d6910757f4af8755b8857dcbe4619cf8d
>
> Please drop Change-ID tag and it would be great how you tested the
> changes, other than that,
>
> Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To add a bit here too. Please explain "why" you're making this
change. Is it because bi_destructor interface is cumbersome? Adding
bi_pool is overhead - why is it justified? Is it because one pointer
is fine to add to struct bio (which I kinda agree) or are there future
changes which will reverse the overhead (which is the case here).
In general, I find the descriptions insufficient. They don't describe
the reasons and reasoning behind the patch.
Thanks.
--
tejun
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Kent Overstreet <koverstreet@google.com>
Cc: axboe@kernel.dk, yehuda@hq.newdream.net, dm-devel@redhat.com,
neilb@suse.de, linux-kernel@vger.kernel.org,
linux-bcache@vger.kernel.org, mpatocka@redhat.com,
vgoyal@redhat.com, bharrosh@panasas.com,
linux-fsdevel@vger.kernel.org, sage@newdream.net, agk@redhat.com,
drbd-dev@lists.linbit.com
Subject: Re: [Drbd-dev] [PATCH v2 01/14] block: Generalized bio pool freeing
Date: Thu, 24 May 2012 09:19:14 -0700 [thread overview]
Message-ID: <20120524161914.GD27983@google.com> (raw)
In-Reply-To: <20120524160944.GB27983@google.com>
On Thu, May 24, 2012 at 09:09:44AM -0700, Tejun Heo wrote:
> On Wed, May 23, 2012 at 05:02:38PM -0700, Kent Overstreet wrote:
> > With the old code, when you allocate a bio from a bio pool you have to
> > implement your own destructor that knows how to find the bio pool the
> > bio was originally allocated from.
> >
> > This adds a new field to struct bio (bi_pool) and changes
> > bio_alloc_bioset() to use it. This makes various bio destructors
> > unnecessary, so they're then deleted.
> >
> > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > Change-Id: I5eb66c1d6910757f4af8755b8857dcbe4619cf8d
>
> Please drop Change-ID tag and it would be great how you tested the
> changes, other than that,
>
> Acked-by: Tejun Heo <tj@kernel.org>
To add a bit here too. Please explain "why" you're making this
change. Is it because bi_destructor interface is cumbersome? Adding
bi_pool is overhead - why is it justified? Is it because one pointer
is fine to add to struct bio (which I kinda agree) or are there future
changes which will reverse the overhead (which is the case here).
In general, I find the descriptions insufficient. They don't describe
the reasons and reasoning behind the patch.
Thanks.
--
tejun
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Kent Overstreet <koverstreet@google.com>
Cc: linux-kernel@vger.kernel.org, linux-bcache@vger.kernel.org,
dm-devel@redhat.com, linux-fsdevel@vger.kernel.org,
axboe@kernel.dk, agk@redhat.com, neilb@suse.de,
drbd-dev@lists.linbit.com, bharrosh@panasas.com,
vgoyal@redhat.com, mpatocka@redhat.com, sage@newdream.net,
yehuda@hq.newdream.net
Subject: Re: [PATCH v2 01/14] block: Generalized bio pool freeing
Date: Thu, 24 May 2012 09:19:14 -0700 [thread overview]
Message-ID: <20120524161914.GD27983@google.com> (raw)
In-Reply-To: <20120524160944.GB27983@google.com>
On Thu, May 24, 2012 at 09:09:44AM -0700, Tejun Heo wrote:
> On Wed, May 23, 2012 at 05:02:38PM -0700, Kent Overstreet wrote:
> > With the old code, when you allocate a bio from a bio pool you have to
> > implement your own destructor that knows how to find the bio pool the
> > bio was originally allocated from.
> >
> > This adds a new field to struct bio (bi_pool) and changes
> > bio_alloc_bioset() to use it. This makes various bio destructors
> > unnecessary, so they're then deleted.
> >
> > Signed-off-by: Kent Overstreet <koverstreet@google.com>
> > Change-Id: I5eb66c1d6910757f4af8755b8857dcbe4619cf8d
>
> Please drop Change-ID tag and it would be great how you tested the
> changes, other than that,
>
> Acked-by: Tejun Heo <tj@kernel.org>
To add a bit here too. Please explain "why" you're making this
change. Is it because bi_destructor interface is cumbersome? Adding
bi_pool is overhead - why is it justified? Is it because one pointer
is fine to add to struct bio (which I kinda agree) or are there future
changes which will reverse the overhead (which is the case here).
In general, I find the descriptions insufficient. They don't describe
the reasons and reasoning behind the patch.
Thanks.
--
tejun
next prev parent reply other threads:[~2012-05-24 16:19 UTC|newest]
Thread overview: 131+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-24 0:02 [PATCH v2 00/14] Block cleanups (for bcache) Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 01/14] block: Generalized bio pool freeing Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
[not found] ` <1337817771-25038-2-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 16:09 ` Tejun Heo
2012-05-24 16:09 ` Tejun Heo
2012-05-24 16:09 ` [Drbd-dev] " Tejun Heo
[not found] ` <20120524160944.GB27983-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 16:19 ` Tejun Heo [this message]
2012-05-24 16:19 ` Tejun Heo
2012-05-24 16:19 ` [Drbd-dev] " Tejun Heo
2012-05-24 17:46 ` Vivek Goyal
2012-05-24 17:46 ` Vivek Goyal
2012-05-24 17:46 ` [Drbd-dev] " Vivek Goyal
[not found] ` <20120524174649.GC27550-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-05-24 18:06 ` Boaz Harrosh
2012-05-24 18:06 ` Boaz Harrosh
2012-05-24 18:06 ` Boaz Harrosh
2012-05-24 18:06 ` [Drbd-dev] " Boaz Harrosh
2012-05-24 0:02 ` [PATCH v2 02/14] dm: kill dm_rq_bio_destructor Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:19 ` [dm-devel] " Jun'ichi Nomura
2012-05-24 0:19 ` [Drbd-dev] " Jun'ichi Nomura
[not found] ` <4FBD7E80.4020005-JhyGz2TFV9J8UrSeD/g0lQ@public.gmane.org>
2012-05-24 0:39 ` Kent Overstreet
2012-05-24 0:39 ` Kent Overstreet
2012-05-24 0:39 ` [Drbd-dev] " Kent Overstreet
[not found] ` <20120524003915.GA27443-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 1:16 ` Jun'ichi Nomura
2012-05-24 1:16 ` Jun'ichi Nomura
2012-05-24 1:16 ` [Drbd-dev] " Jun'ichi Nomura
[not found] ` <4FBD8BD9.8070708-JhyGz2TFV9J8UrSeD/g0lQ@public.gmane.org>
2012-05-24 1:39 ` Jun'ichi Nomura
2012-05-24 1:39 ` Jun'ichi Nomura
2012-05-24 1:39 ` [Drbd-dev] " Jun'ichi Nomura
2012-05-24 23:33 ` Kent Overstreet
2012-05-24 23:33 ` Kent Overstreet
2012-05-24 23:33 ` [Drbd-dev] " Kent Overstreet
[not found] ` <1337817771-25038-3-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 16:11 ` Tejun Heo
2012-05-24 16:11 ` Tejun Heo
2012-05-24 16:11 ` [Drbd-dev] " Tejun Heo
2012-05-24 0:02 ` [PATCH v2 03/14] block: Add bio_clone_bioset() Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 16:38 ` Tejun Heo
2012-05-24 16:38 ` [Drbd-dev] " Tejun Heo
[not found] ` <1337817771-25038-4-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 18:45 ` Vivek Goyal
2012-05-24 18:45 ` Vivek Goyal
2012-05-24 18:45 ` [Drbd-dev] " Vivek Goyal
[not found] ` <20120524184507.GD27550-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-05-24 23:35 ` Kent Overstreet
2012-05-24 23:35 ` Kent Overstreet
2012-05-24 23:35 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 04/14] block: Add bio_clone_kmalloc() Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 18:59 ` Vivek Goyal
2012-05-24 18:59 ` [Drbd-dev] " Vivek Goyal
[not found] ` <20120524185919.GE27550-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-05-24 21:41 ` Yehuda Sadeh
2012-05-24 21:41 ` Yehuda Sadeh
2012-05-24 21:41 ` [Drbd-dev] " Yehuda Sadeh
2012-05-25 0:31 ` Kent Overstreet
2012-05-25 0:31 ` Kent Overstreet
2012-05-25 0:31 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 06/14] block: Add bio_reset() Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 07/14] pktcdvd: Switch to bio_kmalloc() Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
[not found] ` <1337817771-25038-8-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 19:42 ` Vivek Goyal
2012-05-24 19:42 ` Vivek Goyal
2012-05-24 19:42 ` [Drbd-dev] " Vivek Goyal
2012-05-24 19:55 ` Kent Overstreet
2012-05-24 19:55 ` Kent Overstreet
2012-05-24 19:55 ` [Drbd-dev] " Kent Overstreet
[not found] ` <1337817771-25038-1-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 0:02 ` [PATCH v2 05/14] block: Only clone bio vecs that are in use Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 08/14] block: Kill bi_destructor Kent Overstreet
2012-05-24 0:02 ` Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 19:52 ` Vivek Goyal
2012-05-24 19:52 ` [Drbd-dev] " Vivek Goyal
2012-05-24 19:58 ` Vivek Goyal
2012-05-24 19:58 ` [dm-devel] " Vivek Goyal
2012-05-24 19:58 ` [Drbd-dev] " Vivek Goyal
2012-05-24 20:00 ` Kent Overstreet
2012-05-24 20:00 ` [Drbd-dev] " Kent Overstreet
[not found] ` <20120524195202.GG27550-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-05-25 6:43 ` Boaz Harrosh
2012-05-25 6:43 ` Boaz Harrosh
2012-05-25 6:43 ` Boaz Harrosh
2012-05-25 6:43 ` [Drbd-dev] " Boaz Harrosh
2012-05-24 0:02 ` [PATCH v2 09/14] block: Add an explicit bio flag for bios that own their bvec Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
[not found] ` <1337817771-25038-10-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 16:57 ` Boaz Harrosh
2012-05-24 16:57 ` Boaz Harrosh
2012-05-24 16:57 ` Boaz Harrosh
2012-05-24 16:57 ` [Drbd-dev] " Boaz Harrosh
[not found] ` <4FBE687E.1030605-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2012-05-24 21:31 ` Kent Overstreet
2012-05-24 21:31 ` Kent Overstreet
2012-05-24 21:31 ` [Drbd-dev] " Kent Overstreet
[not found] ` <20120524213158.GB22664-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-25 16:49 ` Vivek Goyal
2012-05-25 16:49 ` Vivek Goyal
2012-05-25 16:49 ` [Drbd-dev] " Vivek Goyal
[not found] ` <20120525164914.GE3855-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-05-25 20:01 ` Kent Overstreet
2012-05-25 20:01 ` Kent Overstreet
2012-05-25 20:01 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 10/14] block: Rename bio_split() -> bio_pair_split() Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 11/14] block: Rework bio splitting Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 16:56 ` Boaz Harrosh
2012-05-24 16:56 ` Boaz Harrosh
2012-05-24 16:56 ` [Drbd-dev] " Boaz Harrosh
2012-05-24 21:27 ` Kent Overstreet
2012-05-24 21:27 ` [Drbd-dev] " Kent Overstreet
[not found] ` <4FBE6823.50904-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2012-05-25 18:48 ` Vivek Goyal
2012-05-25 18:48 ` Vivek Goyal
2012-05-25 18:48 ` [Drbd-dev] " Vivek Goyal
2012-05-24 0:02 ` [PATCH v2 12/14] Closures Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:47 ` Joe Perches
2012-05-24 0:47 ` [Drbd-dev] " Joe Perches
2012-05-24 1:16 ` Kent Overstreet
2012-05-24 1:16 ` [Drbd-dev] " Kent Overstreet
[not found] ` <20120524011654.GA28662-RcKxWJ4Cfj3IzGYXcIpNmNLIRw13R84JkQQo+JxHRPFibQn6LdNjmg@public.gmane.org>
2012-05-24 1:23 ` Joe Perches
2012-05-24 1:23 ` Joe Perches
2012-05-24 1:23 ` [Drbd-dev] " Joe Perches
[not found] ` <1337817771-25038-13-git-send-email-koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-05-24 7:39 ` Leen Besselink
2012-05-25 22:54 ` Andi Kleen
2012-05-25 22:54 ` Andi Kleen
2012-05-25 22:54 ` [Drbd-dev] " Andi Kleen
2012-05-24 0:02 ` [PATCH v2 13/14] Make generic_make_request handle arbitrarily large bios Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
2012-05-24 0:02 ` [PATCH v2 14/14] Gut bio_add_page() Kent Overstreet
2012-05-24 0:02 ` [Drbd-dev] " Kent Overstreet
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=20120524161914.GD27983@google.com \
--to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
--cc=agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org \
--cc=bharrosh-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org \
--cc=dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=drbd-dev-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org \
--cc=koverstreet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
--cc=linux-bcache-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mpatocka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=neilb-l3A5Bk7waGM@public.gmane.org \
--cc=sage-BnTBU8nroG7k1uMJSBkQmQ@public.gmane.org \
--cc=vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=yehuda-L5o5AL9CYN0tUFlbccrkMA@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.