| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5df78e1d0912181146h56f76710u721d86edac1f9bd2@mail.gmail.com>
Date: Fri, 18 Dec 2009 11:46:38 -0800
From: Jiaying Zhang <jiayingz@...gle.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc: ext4 development <linux-ext4@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Michael Rubin <mrubin@...gle.com>
Subject: Re: [RFC PATCH 2/4] ext4: use ext4_get_block_write in buffer write
On Fri, Dec 18, 2009 at 1:54 AM, Aneesh Kumar K.V
<aneesh.kumar@...ux.vnet.ibm.com> wrote:
>
> On Tue, Dec 15, 2009 at 05:39:08PM -0800, Jiaying Zhang wrote:
> > ext4: use ext4_get_block_write in buffer write
> >
> > 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@...gle.com>
> > ---
> > fs/ext4/ext4.h | 5 +++
> > fs/ext4/extents.c | 10 +++---
> > fs/ext4/fsync.c | 3 ++
> > fs/ext4/inode.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++--------
> > fs/ext4/super.c | 30 +++++++++++++++++---
> > 5 files changed, 108 insertions(+), 21 deletions(-)
> >
> > Index: git-ext4/fs/ext4/extents.c
> > ===================================================================
> > --- git-ext4.orig/fs/ext4/extents.c 2009-12-15 16:03:05.000000000 -0800
> > +++ git-ext4/fs/ext4/extents.c 2009-12-15 16:03:15.000000000 -0800
> > @@ -3052,6 +3052,7 @@ ext4_ext_handle_uninitialized_extents(ha
> > io->flag = EXT4_IO_UNWRITTEN;
> > else
> > EXT4_I(inode)->i_state |= EXT4_STATE_DIO_UNWRITTEN;
> > + set_buffer_uninit(bh_result);
> > goto out;
> > }
> > /* IO end_io complete, convert the filled extent to written */
> > @@ -3291,11 +3292,9 @@ int ext4_ext_get_blocks(handle_t *handle
> > if (flags & EXT4_GET_BLOCKS_UNINIT_EXT){
> > ext4_ext_mark_uninitialized(&newex);
> > /*
> > - * io_end structure was created for every async
> > - * direct IO write to the middle of the file.
> > - * To avoid unecessary convertion for every aio dio rewrite
> > - * to the mid of file, here we flag the IO that is really
> > - * need the convertion.
> > + * io_end structure was created for every IO write to an
> > + * uninitialized extent. To avoid unecessary convertion,
> > + * here we flag the IO that really needs the convertion.
> > * For non asycn direct IO case, flag the inode state
> > * that we need to perform convertion when IO is done.
> > */
> > @@ -3306,6 +3305,7 @@ int ext4_ext_get_blocks(handle_t *handle
> > EXT4_I(inode)->i_state |=
> > EXT4_STATE_DIO_UNWRITTEN;;
> > }
> > + set_buffer_uninit(bh_result);
> > }
> > err = ext4_ext_insert_extent(handle, inode, path, &newex, flags);
> > if (err) {
> > Index: git-ext4/fs/ext4/ext4.h
> > ===================================================================
> > --- git-ext4.orig/fs/ext4/ext4.h 2009-12-15 16:03:05.000000000 -0800
> > +++ git-ext4/fs/ext4/ext4.h 2009-12-15 16:03:15.000000000 -0800
> > @@ -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 */
> > @@ -752,6 +753,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 */
> > @@ -1764,6 +1766,9 @@ static inline void set_bitmap_uptodate(s
> > set_bit(BH_BITMAP_UPTODATE, &(bh)->b_state);
> > }
> >
> > +/* BH_Uninit flag: blocks are allocated but uninitialized on disk */
> > +#define BH_Uninit (BH_JBDPrivateStart + 1)
> > +BUFFER_FNS(Uninit, uninit)
>
>
> Why do we need to add a new buffer_head flag. Why is unwritten flag not sufficient
> for this ?
>
>
> > #endif /* __KERNEL__ */
> >
> > #endif /* _EXT4_H */
> > Index: git-ext4/fs/ext4/super.c
> > ===================================================================
> > --- git-ext4.orig/fs/ext4/super.c 2009-12-15 16:03:05.000000000 -0800
> > +++ git-ext4/fs/ext4/super.c 2009-12-15 16:03:15.000000000 -0800
> > @@ -921,6 +921,9 @@ static int ext4_show_options(struct seq_
> > if (test_opt(sb, NOLOAD))
> > seq_puts(seq, ",norecovery");
> >
> > + if (test_opt(sb, DIOREAD_NOLOCK))
> > + seq_puts(seq, ",dioread_nolock");
> > +
> > ext4_show_quota_options(seq, sb);
> >
> > return 0;
> > @@ -1103,6 +1106,7 @@ enum {
> > Opt_stripe, Opt_delalloc, Opt_nodelalloc,
> > Opt_block_validity, Opt_noblock_validity,
> > Opt_inode_readahead_blks, Opt_journal_ioprio,
> > + Opt_dioread_nolock, Opt_dioread_lock,
> > Opt_discard, Opt_nodiscard, Opt_akpm_lock_hack
> > };
> >
> > @@ -1171,6 +1175,8 @@ static const match_table_t tokens = {
> > {Opt_auto_da_alloc, "auto_da_alloc=%u"},
> > {Opt_auto_da_alloc, "auto_da_alloc"},
> > {Opt_noauto_da_alloc, "noauto_da_alloc"},
> > + {Opt_dioread_nolock, "dioread_nolock"},
> > + {Opt_dioread_lock, "dioread_lock"},
>
>
>
> I guess this mount option will go away when we are ready to merge this
> upstream ? If we want to merge i guess we should make this the default
> behaviour.
I am not sure yet. It currently only works with bh and data!=journal modes.
I am also not sure whether we want to enable this feature on HDs.
AFAICT, we will see performance improvements only on fast SSDs.
Another concern is that with the patch, we need to allocated uninit extent
first and then do uninit-to-init conversion after the IO is done. So it may
lead to more metadata access, although I didn't see any noticeable
performance difference in my performance testing.
>
> Why version of the kernel are the patches against. I would like to take
> a look at the changes after applying the patches.
I am using a kernel synced with Ted's ext4 git tree two weeks ago.
The kernel version is 2.6.32-rc7.
Jiaying
>
> -aneesh
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists