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 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.