Distributed Replicated Block Device (DRBD) development
 help / color / mirror / Atom feed
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 :

  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