All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: device-mapper development <dm-devel@redhat.com>,
	Zdenek Kabelac <zkabelac@redhat.com>
Subject: Re: mirrored device with thousand of mappingtableentries
Date: Mon, 7 Mar 2011 11:09:14 -0500	[thread overview]
Message-ID: <20110307160914.GA29697@redhat.com> (raw)
In-Reply-To: <yq1y64reiu8.fsf@sermon.lab.mkp.net>

Hi Martin,

On Sun, Mar 06 2011 at  9:59pm -0500,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> >>>>> "Zdenek" == Zdenek Kabelac <zkabelac@redhat.com> writes:
> 
> Zdenek> My finding seems to show that BIP-256 slabtop segment grow by
> Zdenek> ~73KB per each device (while dm-io is ab out ~26KB)
> 
> Ok, I see it now that I tried with a bunch of DM devices.
> 
> DM allocates a bioset per volume. And since each bioset has an integrity
> mempool you'll end up with a bunch of memory locked down. It seems like
> a lot but it's actually the same amount as we reserve for the data path
> (bio-0 + biovec-256).
> 
> Since a bioset is not necessarily tied to a single block device we can't
> automatically decide whether to allocate the integrity pool or not. In
> the DM case, however, we just set up the integrity profile so the
> information is available.
> 
> Can you please try the following patch? This will change things so we
> only attach an integrity pool to the bioset if the logical volume is
> integrity-capable.

Thanks for sorting this out.  Can you post a patch that has a proper
header with your Signed-off-by?  Also including Zdenek's Tested-by, and
my:

Acked-by: Mike Snitzer <snitzer@redhat.com>

Thanks,
Mike

p.s. one small nit:

> diff --git a/fs/bio.c b/fs/bio.c
> index 4bd454f..6e4a381 100644
> --- a/fs/bio.c
> +++ b/fs/bio.c
> @@ -1603,9 +1603,10 @@ void bioset_free(struct bio_set *bs)
>  EXPORT_SYMBOL(bioset_free);
>  
>  /**
> - * bioset_create  - Create a bio_set
> + * bioset_create_flags  - Create a bio_set
>   * @pool_size:	Number of bio and bio_vecs to cache in the mempool
>   * @front_pad:	Number of bytes to allocate in front of the returned bio
> + * @flags:	Flags that affect memory allocation
>   *
>   * Description:
>   *    Set up a bio_set to be used with @bio_alloc_bioset. Allows the caller
> @@ -1615,7 +1616,8 @@ EXPORT_SYMBOL(bioset_free);
>   *    Note that the bio must be embedded at the END of that structure always,
>   *    or things will break badly.
>   */
> -struct bio_set *bioset_create(unsigned int pool_size, unsigned int front_pad)
> +struct bio_set *bioset_create_flags(unsigned int pool_size,
> +				    unsigned int front_pad, unsigned int flags)
>  {
>  	unsigned int back_pad = BIO_INLINE_VECS * sizeof(struct bio_vec);
>  	struct bio_set *bs;
> @@ -1636,7 +1638,8 @@ struct bio_set *bioset_create(unsigned int pool_size, unsigned int front_pad)
>  	if (!bs->bio_pool)
>  		goto bad;
>  
> -	if (bioset_integrity_create(bs, pool_size))
> +	if ((flags & BIOSET_NO_INTEGRITY) == 0 &&
> +	    bioset_integrity_create(bs, pool_size))
>  		goto bad;
>  
>  	if (!biovec_create_pools(bs, pool_size))

We'd generally see: if (!(flags & BIOSET_NO_INTEGRITY) && ...)

but then the double negative is maybe a bit more challenging to deal
with (for a human).  In the end, not a big deal either way ;)

  parent reply	other threads:[~2011-03-07 16:09 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-28 11:21 mirrored device with thousand of mapping table entries Eli Malul
2011-02-28 11:48 ` Alasdair G Kergon
2011-02-28 11:59   ` mirrored device with thousand of mapping tableentries Eli Malul
2011-02-28 12:11     ` Alasdair G Kergon
2011-02-28 12:17       ` mirrored device with thousand of mappingtableentries Eli Malul
2011-02-28 13:10         ` Alasdair G Kergon
2011-02-28 13:13           ` Eli Malul
2011-02-28 13:29             ` Alasdair G Kergon
2011-02-28 13:42               ` Eli Malul
2011-02-28 16:25               ` Eli Malul
2011-02-28 13:38             ` Zdenek Kabelac
2011-02-28 15:01               ` Martin K. Petersen
2011-03-06 20:39                 ` Zdenek Kabelac
2011-03-07  2:59                   ` Martin K. Petersen
2011-03-07 14:24                     ` Zdenek Kabelac
2011-03-07 16:09                     ` Mike Snitzer [this message]
2011-03-08  6:51                       ` Martin K. Petersen
2011-03-08 17:13                         ` Mike Snitzer
2011-03-10 16:11                           ` Martin K. Petersen
2011-03-11 16:53                             ` [PATCH] block: Require subsystems to explicitly allocate bio_set integrity mempool (was: Re: mirrored device with thousand of mappingtableentries) Mike Snitzer
2011-03-11 17:11                               ` [PATCH] block: Require subsystems to explicitly allocate bio_set integrity mempool Martin K. Petersen
2011-03-07 20:10                     ` mirrored device with thousand of mappingtableentries Mike Snitzer
2011-03-07 20:22                       ` Martin K. Petersen

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=20110307160914.GA29697@redhat.com \
    --to=snitzer@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=martin.petersen@oracle.com \
    --cc=zkabelac@redhat.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.