linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mel@csn.ul.ie>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Nick Piggin <npiggin@kernel.dk>, Hugh Dickins <hughd@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [patch] mm/vmalloc: remove block allocation bitmap
Date: Tue, 19 Apr 2011 10:31:18 +0100	[thread overview]
Message-ID: <20110419093118.GB23041@csn.ul.ie> (raw)
In-Reply-To: <20110414211656.GB1700@cmpxchg.org>

On Thu, Apr 14, 2011 at 05:16:56PM -0400, Johannes Weiner wrote:
> Space in a vmap block that was once allocated is considered dirty and
> not made available for allocation again before the whole block is
> recycled.
> 
> The result is that free space within a vmap block is always contiguous
> and the allocation bitmap can be replaced by remembering the offset of
> free space in the block.
> 
> The fragmented block purging was never invoked from vb_alloc() either,
> as it skips blocks that do not have enough free space for the
> allocation in the first place.  According to the above, it is
> impossible for a block to have enough free space and still fail the
> allocation.  Thus, this dead code is removed.  Partially consumed
> blocks will be reclaimed anyway when an attempt is made to allocate a
> new vmap block altogether and no free space is found.
> 
> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Nick Piggin <npiggin@kernel.dk>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Hugh Dickins <hughd@google.com>

I didn't see a problem with the patch per-se but I wonder if your patch
is the intended behaviour. It looks like the intention was that dirty
blocks could be flushed from the TLB and made available for allocations
leading to the possibility of fragmented vmap blocks.

It's this check that is skipping over blocks without taking dirty into
account.

  		spin_lock(&vb->lock);
 		if (vb->free < 1UL << order)
 			goto next;

It was introduced by [02b709d: mm: purge fragmented percpu vmap blocks]
but is there any possibility that this is what should be fixed instead?
Do we know what the consequences of blocks only getting flushed when
they have been fully allocated are?

> <SNIP>

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-04-19 11:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-14 21:16 Johannes Weiner
2011-04-19  9:31 ` Mel Gorman [this message]
2011-04-19 23:39   ` Johannes Weiner
2011-04-20  9:46     ` Mel Gorman
2011-04-20 16:39       ` Christoph Hellwig
2011-04-23  2:08       ` Johannes Weiner
2011-04-25 11:08         ` Mel Gorman

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=20110419093118.GB23041@csn.ul.ie \
    --to=mel@csn.ul.ie \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@kernel.dk \
    /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