From: Philipp Reisner <philipp.reisner@linbit.com>
To: drbd-dev@lists.linbit.com
Subject: Re: [Drbd-dev] Re: [DRBD-cvs] r1743 - in branches/drbd-0.7: . drbd
Date: Tue, 1 Feb 2005 18:36:43 +0100 [thread overview]
Message-ID: <200502011836.43449.philipp.reisner@linbit.com> (raw)
In-Reply-To: <20050201164820.GL7628@marowsky-bree.de>
Am Dienstag, 1. Februar 2005 17:48 schrieb Lars Marowsky-Bree:
> On 2005-01-31T12:19:21, svn@svn.drbd.org wrote:
> > Modified:
> > branches/drbd-0.7/ChangeLog
> > branches/drbd-0.7/drbd.spec.in
> > branches/drbd-0.7/drbd/drbd_actlog.c
> > branches/drbd-0.7/drbd/drbd_compat_wrappers.h
> > Log:
> > No longer leak of BIOs.
>
> Good goal, but ...
>
> > Modified: branches/drbd-0.7/drbd/drbd_actlog.c
> > ===================================================================
> > --- branches/drbd-0.7/drbd/drbd_actlog.c 2005-01-31 09:20:39 UTC (rev
> > 1742) +++ branches/drbd-0.7/drbd/drbd_actlog.c 2005-01-31 11:19:19 UTC
> > (rev 1743) @@ -68,8 +68,6 @@
> > struct completion event;
> > int ok;
> >
> > - bio_get(bio);
> > -
> > bio->bi_bdev = mdev->md_bdev;
> > bio->bi_sector = sector;
> > bio_add_page(bio, page, size, 0);
>
> I think this is a bug. If you look at the comments in
> linux/include/linux/bio.h, you'll find that this is just how the bio is
> supposed to be used:
>
> /*
> * get a reference to a bio, so it won't disappear. the intended use is
> * something like:
> *
> * bio_get(bio);
> * submit_bio(rw, bio);
> * if (bio->bi_flags ...)
> * do_something
> * bio_put(bio);
> *
> * without the bio_get(), it could potentially complete I/O before
> submit_bio * returns. and then bio would be freed memory when if
> (bio->bi_flags ...) * runs
> */
> #define bio_get(bio) atomic_inc(&(bio)->bi_cnt)
>
> The fix I did was not the right way (removing the bio embedding was much
> better), but it also wasn't completely wrong.
>
I did not look at the comments, I read the code. And what I read there was:
a BIO has after bio_init() a refcount (bi_cnt) of 1. It is freed when the
refcount drops to zero.
bio_alloc();
do something with it
bio_put();
is right.
What you did is:
bio_alloc();
bio_get(); // increase to 2
bio_put(); // decreat to 1
and let it live forever...
-Philipp
--
: Dipl-Ing Philipp Reisner Tel +43-1-8178292-50 :
: LINBIT Information Technologies GmbH Fax +43-1-8178292-82 :
: Schönbrunnerstr 244, 1120 Vienna, Austria http://www.linbit.com :
next prev parent reply other threads:[~2005-02-01 17:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20050131111921.EE6F83BE6D@garcon.linbit.com>
2005-02-01 16:48 ` [Drbd-dev] Re: [DRBD-cvs] r1743 - in branches/drbd-0.7: . drbd Lars Marowsky-Bree
2005-02-01 17:36 ` Philipp Reisner [this message]
2005-02-02 10:13 ` Lars Marowsky-Bree
2005-02-02 17:59 ` Philipp Reisner
2005-02-03 8:38 ` Jens Axboe
2005-02-03 9:15 ` Lars Marowsky-Bree
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=200502011836.43449.philipp.reisner@linbit.com \
--to=philipp.reisner@linbit.com \
--cc=drbd-dev@lists.linbit.com \
/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