| 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
| ||
|
Date: Wed, 13 Aug 2008 17:50:56 -0700
From: Mingming Cao <cmm@...ibm.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
Cc: tytso <tytso@....edu>, linux-ext4@...r.kernel.org,
Andreas Dilger <adilger@....com>
Subject: Re: [PATCH 3/6 ]Ext4: journal credits reservation fixes for DIO,
fallocate
在 2008-08-13三的 14:23 +0530，Aneesh Kumar K.V写道：
> On Tue, Aug 12, 2008 at 09:29:50AM -0700, Mingming Cao wrote:
> >
> ......
> ....
>
> >
> > ===================================================================
> > Index: linux-2.6.27-rc1/fs/ext4/extents.c
> > ===================================================================
> > --- linux-2.6.27-rc1.orig/fs/ext4/extents.c 2008-08-11 22:25:39.000000000 -0700
> > +++ linux-2.6.27-rc1/fs/ext4/extents.c 2008-08-11 22:25:55.000000000 -0700
> > @@ -2799,7 +2799,7 @@ void ext4_ext_truncate(struct inode *ino
> > /*
> > * probably first extent we're gonna free will be last in block
> > */
> > - err = ext4_writepage_trans_blocks(inode) + 3;
> > + err = ext4_writepage_trans_blocks(inode);
> > handle = ext4_journal_start(inode, err);
> > if (IS_ERR(handle))
> > return;
> > @@ -2951,10 +2951,9 @@ long ext4_fallocate(struct inode *inode,
> > max_blocks = (EXT4_BLOCK_ALIGN(len + offset, blkbits) >> blkbits)
> > - block;
> > /*
> > - * credits to insert 1 extent into extent tree + buffers to be able to
> > - * modify 1 super block, 1 block bitmap and 1 group descriptor.
> > + * credits to insert 1 extent into extent tree
> > */
> > - credits = EXT4_DATA_TRANS_BLOCKS(inode->i_sb) + 3;
> > + credits = ext4_data_trans_blocks(inode, max_blocks);
>
>
> Why do we need to consider data=journaled mode here. We are not writing
> any data here. Instead we are just inserting an extent.
>
Actually the change here is not mean to support data=journalled here.
The ext4_data_trans_blocks() is intended for calculate credits for a
chunk of allocation, used for DIO and fallocate, regardless of delalloc
or not.
We should remove the considering of data journal in the
ext4_data_trans_blocks(), I agree.
Now that I realize the data=journalled code doesn't work for delalloc
(or delalloc da writepages doesn' t support the journalled mode, due to
the lock ordering issue), I am not sure if there is plan to do so (or
there is need to support journalled mode on delalloc). We still need to
keep the data=journalled consideration for writepage/write_begin, just
to help user move from ext3 to ext4 I guess.
>
> > mutex_lock(&inode->i_mutex);
> > retry:
> > while (ret >= 0 && ret < max_blocks) {
> > Index: linux-2.6.27-rc1/fs/ext4/inode.c
> > ===================================================================
> > --- linux-2.6.27-rc1.orig/fs/ext4/inode.c 2008-08-11 22:18:31.000000000 -0700
> > +++ linux-2.6.27-rc1/fs/ext4/inode.c 2008-08-11 22:25:55.000000000 -0700
> > @@ -1041,18 +1041,6 @@ static void ext4_da_update_reserve_space
> > spin_unlock(&EXT4_I(inode)->i_block_reservation_lock);
> > }
> >
> > -/* Maximum number of blocks we map for direct IO at once. */
> > -#define DIO_MAX_BLOCKS 4096
> > -/*
> > - * Number of credits we need for writing DIO_MAX_BLOCKS:
> > - * We need sb + group descriptor + bitmap + inode -> 4
> > - * For B blocks with A block pointers per block we need:
> > - * 1 (triple ind.) + (B/A/A + 2) (doubly ind.) + (B/A + 2) (indirect).
> > - * If we plug in 4096 for B and 256 for A (for 1KB block size), we get 25.
> > - */
> > -#define DIO_CREDITS 25
> > -
> > -
> > /*
> > * The ext4_get_blocks_wrap() function try to look up the requested blocks,
> > * and returns if the blocks are already mapped.
> > @@ -1164,19 +1152,23 @@ int ext4_get_blocks_wrap(handle_t *handl
> > return retval;
> > }
> >
> > +/* Maximum number of blocks we map for direct IO at once. */
> > +#define DIO_MAX_BLOCKS 4096
> > +
> > static int ext4_get_block(struct inode *inode, sector_t iblock,
> > struct buffer_head *bh_result, int create)
> > {
> > handle_t *handle = ext4_journal_current_handle();
> > int ret = 0, started = 0;
> > unsigned max_blocks = bh_result->b_size >> inode->i_blkbits;
> > + int dio_credits;
> >
> > if (create && !handle) {
> > /* Direct IO write... */
> > if (max_blocks > DIO_MAX_BLOCKS)
> > max_blocks = DIO_MAX_BLOCKS;
> > - handle = ext4_journal_start(inode, DIO_CREDITS +
> > - 2 * EXT4_QUOTA_TRANS_BLOCKS(inode->i_sb));
> > + dio_credits = ext4_data_trans_blocks(inode, max_blocks);
> > + handle = ext4_journal_start(inode, dio_credits);
>
> Even in data=journal mode directIO will put the buffer_heads to journal
> right ? . So should we use ext4_data_trans_blocks here ?
>
>
>
> > if (IS_ERR(handle)) {
> > ret = PTR_ERR(handle);
> > goto out;
> > @@ -2222,7 +2214,7 @@ static int ext4_da_writepage(struct page
> > * for DIO, writepages, and truncate
> > */
> > #define EXT4_MAX_WRITEBACK_PAGES DIO_MAX_BLOCKS
> > -#define EXT4_MAX_WRITEBACK_CREDITS DIO_CREDITS
> > +#define EXT4_MAX_WRITEBACK_CREDITS 25
> >
> > static int ext4_da_writepages(struct address_space *mapping,
> > struct writeback_control *wbc)
> > @@ -4429,7 +4421,8 @@ static int ext4_writeblocks_trans_credit
> >
> > /*
> >
>
> ....
> ....
>
> -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