Re: [PATCH v4 2/3] ext4: use ext4_get_block_write in buffer write

[Jiaying Zhang <jiayingz@google.com>]

I agree that unwritten flag would be a better choice. I was thinking
to use it at the beginning but found it would be tricky to get it work.
See e.g. the unwritten flag usage in the current ext4_get_block.
I guess at some time later, we should clean up the buffer head
flag usage.

Jiaying

On Sun, Jan 17, 2010 at 6:21 AM, Aneesh Kumar K. V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
>
> On Fri, 15 Jan 2010 14:30:11 -0500, "Theodore Ts'o" <tytso@mit.edu> wrote:
> > Allocate uninitialized extent before ext4 buffer write and
> > convert the extent to initialized after io completes.
> > The purpose is to make sure an extent can only be marked
> > initialized after it has been written with new data so
> > we can safely drop the i_mutex lock in ext4 DIO read without
> > exposing stale data. This helps to improve multi-thread DIO
> > read performance on high-speed disks.
> >
> > Skip the nobh and data=journal mount cases to make things simple for now.
> >
> > Signed-off-by: Jiaying Zhang <jiayingz@google.com>
> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
> > ---
> >  fs/ext4/ext4.h      |   12 +++++-
> >  fs/ext4/ext4_jbd2.h |   24 ++++++++++++
> >  fs/ext4/extents.c   |   22 ++++++-----
> >  fs/ext4/inode.c     |  105 ++++++++++++++++++++++++++++++++++++++++----------
> >  fs/ext4/super.c     |   30 +++++++++++++--
> >  5 files changed, 157 insertions(+), 36 deletions(-)
> >
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index b1dcbb7..b8b4887 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -134,6 +134,7 @@ struct mpage_da_data {
> >       int retval;
> >  };
> >  #define      EXT4_IO_UNWRITTEN       0x1
> > +#define      EXT4_IO_WRITTEN         0x2
> >  typedef struct ext4_io_end {
> >       struct list_head        list;           /* per-file finished AIO list */
> >       struct inode            *inode;         /* file being written to */
> > @@ -370,7 +371,7 @@ struct ext4_new_group_data {
> >                                        EXT4_GET_BLOCKS_CREATE_UNINIT_EXT)
> >       /* Convert extent to initialized after IO complete */
> >  #define EXT4_GET_BLOCKS_IO_CONVERT_EXT               (EXT4_GET_BLOCKS_CONVERT|\
> > -                                      EXT4_GET_BLOCKS_IO_CREATE_EXT)
> > +                                      EXT4_GET_BLOCKS_CREATE_UNINIT_EXT)
> >
> >  /*
> >   * Flags used by ext4_free_blocks
> > @@ -761,6 +762,7 @@ struct ext4_inode_info {
> >  #define EXT4_MOUNT_QUOTA             0x80000 /* Some quota option set */
> >  #define EXT4_MOUNT_USRQUOTA          0x100000 /* "old" user quota */
> >  #define EXT4_MOUNT_GRPQUOTA          0x200000 /* "old" group quota */
> > +#define EXT4_MOUNT_DIOREAD_NOLOCK    0x400000 /* Enable support for dio read nolocking */
> >  #define EXT4_MOUNT_JOURNAL_CHECKSUM  0x800000 /* Journal checksums */
> >  #define EXT4_MOUNT_JOURNAL_ASYNC_COMMIT      0x1000000 /* Journal Async Commit */
> >  #define EXT4_MOUNT_I_VERSION            0x2000000 /* i_version support */
> > @@ -1774,6 +1776,14 @@ static inline void set_bitmap_uptodate(struct buffer_head *bh)
> >       set_bit(BH_BITMAP_UPTODATE, &(bh)->b_state);
> >  }
> >
> > +/* BH_Uninit flag: blocks are allocated but uninitialized on disk */
> > +enum ext4_state_bits {
> > +     BH_Uninit       /* blocks are allocated but uninitialized on disk */
> > +       = BH_JBDPrivateStart,
> > +};
> > +
> > +BUFFER_FNS(Uninit, uninit)
> > +
>
>
> I asked this in the last post. Why we need a new buffer head flag ?
> Why can't we use the unwritten flag ?
>
> -aneesh
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html