linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
To: Jan Kara <jack@suse.cz>
Cc: LKML <linux-kernel@vger.kernel.org>,
	npiggin@suse.de, linux-ext4@vger.kernel.org
Subject: Re: [PATCH 08/11] vfs: Unmap underlying metadata of new data buffers only when buffer is mapped
Date: Thu, 28 May 2009 15:45:54 +0530	[thread overview]
Message-ID: <20090528101554.GB27978@skywalker> (raw)
In-Reply-To: <20090528094434.GG29199@duck.suse.cz>

On Thu, May 28, 2009 at 11:44:34AM +0200, Jan Kara wrote:
> On Wed 27-05-09 21:05:59, Aneesh Kumar K.V wrote:
> > On Wed, May 27, 2009 at 03:01:05PM +0200, Jan Kara wrote:
> > > When we do delayed allocation of some buffer, we want to signal to VFS that
> > > the buffer is new (set buffer_new) so that it properly zeros out everything.
> > > But we don't have the buffer mapped yet so we cannot really unmap underlying
> > > metadata in this state. Make VFS avoid doing unmapping of metadata when the
> > > buffer is not yet mapped.
> > > 
> ...
> > > @@ -2683,7 +2685,7 @@ int nobh_write_begin(struct file *file, struct address_space *mapping,
> > >  			goto failed;
> > >  		if (!buffer_mapped(bh))
> > >  			is_mapped_to_disk = 0;
> > > -		if (buffer_new(bh))
> > > +		if (buffer_new(bh) && buffer_mapped(bh))
> > >  			unmap_underlying_metadata(bh->b_bdev, bh->b_blocknr);
> > >  		if (PageUptodate(page)) {
> > >  			set_buffer_uptodate(bh);
> > 
> > Both xfs and ext4 return mapped delay buffer_head when we do a get_block
> > with delayed allocation in write_begin phase.
>   Yeah, I knew about ext4 doing this. Thanks for pointing this out. I
> wanted to trigger a separate discussion about this and similar problems -
> the current state of buffer bits is quite messy (I think Ted complained
> about this as well recently) and we should somehow clean it up.
>   In this particular case: What's the point in returning the buffer mapped?
> It does not make any sence logically (block *does not* have any physical
> location assigned) and technically you have to map it to some fake block
> and later remap it correctly when you do block allocation. So maybe I'm
> missing some good reason but from what I can see, it just does not make
> sence...

Marking it mapped make sure we don't do multiple get_block calls for
every write. For each write in write_begin path we do a get_block
call if the buffer is not mapped. (__block_prepare_write have more
details.)

-aneesh

  reply	other threads:[~2009-05-28 10:15 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-27 13:00 [PATCH 0/11] Fix page_mkwrite() for blocksize < pagesize Jan Kara
2009-05-27 13:00 ` [PATCH 01/11] ext3: Get rid of extenddisksize parameter of ext3_get_blocks_handle() Jan Kara
2009-05-27 13:00 ` [PATCH 02/11] ext4: Get rid of extend_disksize parameter of ext4_get_blocks_handle() Jan Kara
2009-05-27 13:01 ` [PATCH 03/11] vfs: Add better VFS support for page_mkwrite when blocksize < pagesize Jan Kara
2009-05-27 16:00   ` Josef Bacik
2009-05-27 16:45     ` Aneesh Kumar K.V
2009-05-27 17:06       ` Josef Bacik
2009-05-28 13:03   ` Josef Bacik
2009-05-28 13:10     ` Aneesh Kumar K.V
2009-05-30 11:23   ` Pavel Machek
2009-06-01  9:44     ` Jan Kara
2009-06-01 11:33       ` Goswin von Brederlow
2009-06-01 14:00         ` Jan Kara
2009-06-01 14:46           ` Goswin von Brederlow
2009-06-01 15:02             ` Jan Kara
2009-06-01 15:35               ` Goswin von Brederlow
2009-05-27 13:01 ` [PATCH 04/11] ext2: Allocate space for mmaped file on page fault Jan Kara
2009-05-27 13:01 ` [PATCH 05/11] ext4: Make sure blocks are properly allocated under mmaped page even when blocksize < pagesize Jan Kara
2009-05-27 14:30   ` Theodore Tso
2009-05-27 14:52     ` Jan Kara
2009-06-04 14:09   ` Theodore Tso
2009-05-27 13:01 ` [PATCH 06/11] ext3: Allocate space for mmaped file on page fault Jan Kara
2009-05-27 13:01 ` [PATCH 07/11] vfs: Implement generic per-cpu counters for delayed allocation Jan Kara
2009-05-27 13:01 ` [PATCH 08/11] vfs: Unmap underlying metadata of new data buffers only when buffer is mapped Jan Kara
2009-05-27 15:35   ` Aneesh Kumar K.V
2009-05-28  9:44     ` Jan Kara
2009-05-28 10:15       ` Aneesh Kumar K.V [this message]
2009-05-28 13:50         ` Jan Kara
2009-05-27 13:01 ` [PATCH 09/11] fs: Don't clear dirty bits in block_write_full_page() Jan Kara
2009-05-27 13:01 ` [PATCH 10/11] vfs: Export wakeup_pdflush Jan Kara
2009-05-27 13:01 ` [PATCH 11/11] ext3: Implement delayed allocation on page_mkwrite time Jan Kara
2009-05-27 14:23 ` [PATCH 0/11] Fix page_mkwrite() for blocksize < pagesize Theodore Tso
2009-05-27 14:59   ` Jan Kara
2009-06-04 17:11     ` Theodore Tso
2009-06-05 23:23       ` Jan Kara
2009-05-27 15:33 ` Aneesh Kumar K.V
2009-05-28  9:36   ` Jan Kara

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=20090528101554.GB27978@skywalker \
    --to=aneesh.kumar@linux.vnet.ibm.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@suse.de \
    /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;
as well as URLs for NNTP newsgroup(s).