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: <20130210084511.GB14075@gmail.com> Date: Sun, 10 Feb 2013 16:45:11 +0800 From: Zheng Liu <gnehzuil.liu@...il.com> To: Theodore Ts'o <tytso@....edu> Cc: Zheng Liu <wenqing.lz@...bao.com>, Jan kara <jack@...e.cz>, linux-ext4@...r.kernel.org Subject: Re: [PATCH 09/10 v5] ext4: convert unwritten extents from extent status tree in end_io On Fri, Feb 08, 2013 at 04:44:05PM +0800, Zheng Liu wrote: > From: Zheng Liu <wenqing.lz@...bao.com> > > This commit tries to convert unwritten extents from extent status tree > in end_io callback functions and ext4_ext_direct_IO. > > Signed-off-by: Zheng Liu <wenqing.lz@...bao.com> > Cc: "Theodore Ts'o" <tytso@....edu> > Cc: Jan kara <jack@...e.cz> > --- > fs/ext4/extents.c | 6 +- > fs/ext4/extents_status.c | 180 ++++++++++++++++++++++++++++++++++++++++---- > fs/ext4/extents_status.h | 2 + > fs/ext4/inode.c | 5 ++ > fs/ext4/page-io.c | 8 +- > include/trace/events/ext4.h | 25 ++++++ > 6 files changed, 208 insertions(+), 18 deletions(-) > > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index 9f21430..a03cabf 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -4443,8 +4443,10 @@ int ext4_convert_unwritten_extents(struct inode *inode, loff_t offset, > ret = PTR_ERR(handle); > break; > } > - ret = ext4_map_blocks(handle, inode, &map, > - EXT4_GET_BLOCKS_IO_CONVERT_EXT); > + down_write(&EXT4_I(inode)->i_data_sem); > + ret = ext4_ext_map_blocks(handle, inode, &map, > + EXT4_GET_BLOCKS_IO_CONVERT_EXT); > + up_write(&EXT4_I(inode)->i_data_sem); > if (ret <= 0) { > WARN_ON(ret <= 0); > ext4_msg(inode->i_sb, KERN_ERR, Hi Ted, This chunk is missing in your 'dev' branch of ext4. If we call ext4_map_blocks here, unwritten extent in disk will never be converted because ext4_map_blocks first tries to lookup extent status tree and the unwriten extent in this tree has been converted in end_io callback function. It always looks up a written extent in cache, and return immdiately. Please check it again. Thanks, - Zheng > diff --git a/fs/ext4/extents_status.c b/fs/ext4/extents_status.c > index bac5286..eab8893 100644 > --- a/fs/ext4/extents_status.c > +++ b/fs/ext4/extents_status.c > @@ -248,10 +248,11 @@ ext4_lblk_t ext4_es_find_delayed_extent(struct inode *inode, > struct extent_status *es1 = NULL; > struct rb_node *node; > ext4_lblk_t ret = EXT_MAX_BLOCKS; > + unsigned long flags; > > trace_ext4_es_find_delayed_extent_enter(inode, es->es_lblk); > > - read_lock(&EXT4_I(inode)->i_es_lock); > + read_lock_irqsave(&EXT4_I(inode)->i_es_lock, flags); > tree = &EXT4_I(inode)->i_es_tree; > > /* find extent in cache firstly */ > @@ -291,7 +292,7 @@ out: > } > } > > - read_unlock(&EXT4_I(inode)->i_es_lock); > + read_unlock_irqrestore(&EXT4_I(inode)->i_es_lock, flags); > > ext4_es_lru_add(inode); > trace_ext4_es_find_delayed_extent_exit(inode, es, ret); > @@ -458,6 +459,7 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk, > { > struct extent_status newes; > ext4_lblk_t end = lblk + len - 1; > + unsigned long flags; > int err = 0; > > es_debug("add [%u/%u) %llu %llx to extent status tree of inode %lu\n", > @@ -471,14 +473,14 @@ int ext4_es_insert_extent(struct inode *inode, ext4_lblk_t lblk, > ext4_es_store_status(&newes, status); > trace_ext4_es_insert_extent(inode, &newes); > > - write_lock(&EXT4_I(inode)->i_es_lock); > + write_lock_irqsave(&EXT4_I(inode)->i_es_lock, flags); > err = __es_remove_extent(inode, lblk, end); > if (err != 0) > goto error; > err = __es_insert_extent(inode, &newes); > > error: > - write_unlock(&EXT4_I(inode)->i_es_lock); > + write_unlock_irqrestore(&EXT4_I(inode)->i_es_lock, flags); > > ext4_es_lru_add(inode); > ext4_es_print_tree(inode); > @@ -498,13 +500,14 @@ int ext4_es_lookup_extent(struct inode *inode, struct extent_status *es) > struct ext4_es_tree *tree; > struct extent_status *es1 = NULL; > struct rb_node *node; > + unsigned long flags; > int found = 0; > > trace_ext4_es_lookup_extent_enter(inode, es->es_lblk); > es_debug("lookup extent in block %u\n", es->es_lblk); > > tree = &EXT4_I(inode)->i_es_tree; > - read_lock(&EXT4_I(inode)->i_es_lock); > + read_lock_irqsave(&EXT4_I(inode)->i_es_lock, flags); > > /* find extent in cache firstly */ > es->es_len = es->es_pblk = 0; > @@ -539,7 +542,7 @@ out: > es->es_pblk = es1->es_pblk; > } > > - read_unlock(&EXT4_I(inode)->i_es_lock); > + read_unlock_irqrestore(&EXT4_I(inode)->i_es_lock, flags); > > ext4_es_lru_add(inode); > trace_ext4_es_lookup_extent_exit(inode, es, found); > @@ -649,6 +652,7 @@ int ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk, > ext4_lblk_t len) > { > ext4_lblk_t end; > + unsigned long flags; > int err = 0; > > trace_ext4_es_remove_extent(inode, lblk, len); > @@ -658,9 +662,9 @@ int ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk, > end = lblk + len - 1; > BUG_ON(end < lblk); > > - write_lock(&EXT4_I(inode)->i_es_lock); > + write_lock_irqsave(&EXT4_I(inode)->i_es_lock, flags); > err = __es_remove_extent(inode, lblk, end); > - write_unlock(&EXT4_I(inode)->i_es_lock); > + write_unlock_irqrestore(&EXT4_I(inode)->i_es_lock, flags); > ext4_es_print_tree(inode); > return err; > } > @@ -671,6 +675,7 @@ static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc) > struct ext4_sb_info, s_es_shrinker); > struct ext4_inode_info *ei; > struct list_head *cur, *tmp, scanned; > + unsigned long flags; > int nr_to_scan = sc->nr_to_scan; > int ret, nr_shrunk = 0; > > @@ -687,16 +692,16 @@ static int ext4_es_shrink(struct shrinker *shrink, struct shrink_control *sc) > > ei = list_entry(cur, struct ext4_inode_info, i_es_lru); > > - read_lock(&ei->i_es_lock); > + read_lock_irqsave(&ei->i_es_lock, flags); > if (ei->i_es_lru_nr == 0) { > - read_unlock(&ei->i_es_lock); > + read_unlock_irqrestore(&ei->i_es_lock, flags); > continue; > } > - read_unlock(&ei->i_es_lock); > + read_unlock_irqrestore(&ei->i_es_lock, flags); > > - write_lock(&ei->i_es_lock); > + write_lock_irqsave(&ei->i_es_lock, flags); > ret = __es_try_to_reclaim_extents(ei, nr_to_scan); > - write_unlock(&ei->i_es_lock); > + write_unlock_irqrestore(&ei->i_es_lock, flags); > > nr_shrunk += ret; > nr_to_scan -= ret; > @@ -756,14 +761,15 @@ static int ext4_es_reclaim_extents_count(struct super_block *sb) > struct ext4_sb_info *sbi = EXT4_SB(sb); > struct ext4_inode_info *ei; > struct list_head *cur; > + unsigned long flags; > int nr_cached = 0; > > spin_lock(&sbi->s_es_lru_lock); > list_for_each(cur, &sbi->s_es_lru) { > ei = list_entry(cur, struct ext4_inode_info, i_es_lru); > - read_lock(&ei->i_es_lock); > + read_lock_irqsave(&ei->i_es_lock, flags); > nr_cached += ei->i_es_lru_nr; > - read_unlock(&ei->i_es_lock); > + read_unlock_irqrestore(&ei->i_es_lock, flags); > } > spin_unlock(&sbi->s_es_lru_lock); > trace_ext4_es_reclaim_extents_count(sb, nr_cached); > @@ -801,3 +807,147 @@ static int __es_try_to_reclaim_extents(struct ext4_inode_info *ei, > tree->cache_es = NULL; > return nr_shrunk; > } > + > +int ext4_es_convert_unwritten_extents(struct inode *inode, loff_t offset, > + size_t size) > +{ > + struct ext4_es_tree *tree; > + struct rb_node *node; > + struct extent_status *es, orig_es, conv_es; > + ext4_lblk_t end, len1, len2; > + ext4_lblk_t lblk = 0, len = 0; > + ext4_fsblk_t block; > + unsigned long flags; > + unsigned int blkbits; > + int err = 0; > + > + trace_ext4_es_convert_unwritten_extents(inode, offset, size); > + blkbits = inode->i_blkbits; > + lblk = offset >> blkbits; > + len = (EXT4_BLOCK_ALIGN(offset + size, blkbits) >> blkbits) - lblk; > + > + end = lblk + len - 1; > + BUG_ON(end < lblk); > + > + tree = &EXT4_I(inode)->i_es_tree; > + > + write_lock_irqsave(&EXT4_I(inode)->i_es_lock, flags); > + es = __es_tree_search(&tree->root, lblk); > + if (!es) > + goto out; > + if (es->es_lblk > end) > + goto out; > + > + tree->cache_es = NULL; > + > + orig_es.es_lblk = es->es_lblk; > + orig_es.es_len = es->es_len; > + orig_es.es_pblk = es->es_pblk; > + > + len1 = lblk > es->es_lblk ? lblk - es->es_lblk : 0; > + len2 = ext4_es_end(es) > end ? > + ext4_es_end(es) - end : 0; > + if (len1 > 0) > + es->es_len = len1; > + if (len2 > 0) { > + if (len1 > 0) { > + struct extent_status newes; > + > + newes.es_lblk = end + 1; > + newes.es_len = len2; > + block = ext4_es_pblock(&orig_es) + > + orig_es.es_len - len2; > + ext4_es_store_pblock(&newes, block); > + ext4_es_store_status(&newes, ext4_es_status(&orig_es)); > + err = __es_insert_extent(inode, &newes); > + if (err) { > + es->es_lblk = orig_es.es_lblk; > + es->es_len = orig_es.es_len; > + es->es_pblk = orig_es.es_pblk; > + goto out; > + } > + > + conv_es.es_lblk = orig_es.es_lblk + len1; > + conv_es.es_len = orig_es.es_len - len1 - len2; > + block = ext4_es_pblock(&orig_es) + len1; > + ext4_es_store_pblock(&conv_es, block); > + ext4_es_store_status(&conv_es, EXTENT_STATUS_WRITTEN); > + err = __es_insert_extent(inode, &conv_es); > + if (err) { > + int err2 = __es_remove_extent(inode, > + conv_es.es_lblk, > + ext4_es_end(&newes)); > + if (err2) > + goto out; > + es->es_lblk = orig_es.es_lblk; > + es->es_len = orig_es.es_len; > + es->es_pblk = orig_es.es_pblk; > + goto out; > + } > + } else { > + es->es_lblk = end + 1; > + es->es_len = len2; > + block = ext4_es_pblock(&orig_es) + > + orig_es.es_len - len2; > + ext4_es_store_pblock(es, block); > + > + conv_es.es_lblk = orig_es.es_lblk; > + conv_es.es_len = orig_es.es_len - len2; > + ext4_es_store_pblock(&conv_es, > + ext4_es_pblock(&orig_es)); > + ext4_es_store_status(&conv_es, EXTENT_STATUS_WRITTEN); > + err = __es_insert_extent(inode, &conv_es); > + if (err) { > + es->es_lblk = orig_es.es_lblk; > + es->es_len = orig_es.es_len; > + es->es_pblk = orig_es.es_pblk; > + } > + } > + goto out; > + } > + > + if (len1 > 0) { > + node = rb_next(&es->rb_node); > + if (node) > + es = rb_entry(node, struct extent_status, rb_node); > + else > + es = NULL; > + } > + > + while (es && ext4_es_end(es) <= end) { > + node = rb_next(&es->rb_node); > + ext4_es_store_status(es, EXTENT_STATUS_WRITTEN); > + if (!inode) { > + es = NULL; > + break; > + } > + es = rb_entry(node, struct extent_status, rb_node); > + } > + > + if (es && es->es_lblk < end + 1) { > + ext4_lblk_t orig_len = es->es_len; > + > + /* > + * Here we first set conv_es just because of avoiding copy the > + * value of es to a temporary variable. > + */ > + len1 = ext4_es_end(es) - end; > + conv_es.es_lblk = es->es_lblk; > + conv_es.es_len = es->es_len - len1; > + ext4_es_store_pblock(&conv_es, ext4_es_pblock(es)); > + ext4_es_store_status(&conv_es, EXTENT_STATUS_WRITTEN); > + > + es->es_lblk = end + 1; > + es->es_len = len1; > + block = ext4_es_pblock(es) + orig_len - len1; > + ext4_es_store_pblock(es, block); > + > + err = __es_insert_extent(inode, &conv_es); > + if (err) > + goto out; > + } > + > +out: > + write_unlock_irqrestore(&EXT4_I(inode)->i_es_lock, flags); > + return err; > +} > diff --git a/fs/ext4/extents_status.h b/fs/ext4/extents_status.h > index 938ad2b..2849d74 100644 > --- a/fs/ext4/extents_status.h > +++ b/fs/ext4/extents_status.h > @@ -54,6 +54,8 @@ extern int ext4_es_remove_extent(struct inode *inode, ext4_lblk_t lblk, > extern ext4_lblk_t ext4_es_find_delayed_extent(struct inode *inode, > struct extent_status *es); > extern int ext4_es_lookup_extent(struct inode *inode, struct extent_status *es); > +extern int ext4_es_convert_unwritten_extents(struct inode *inode, loff_t offset, > + size_t size); > > static inline int ext4_es_is_written(struct extent_status *es) > { > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 670779a..08cf720 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -3063,6 +3063,7 @@ out: > io_end->result = ret; > } > > + ext4_es_convert_unwritten_extents(inode, offset, size); > ext4_add_complete_io(io_end); > } > > @@ -3088,6 +3089,7 @@ static void ext4_end_io_buffer_write(struct buffer_head *bh, int uptodate) > */ > inode = io_end->inode; > ext4_set_io_unwritten_flag(inode, io_end); > + ext4_es_convert_unwritten_extents(inode, io_end->offset, io_end->size); > ext4_add_complete_io(io_end); > out: > bh->b_private = NULL; > @@ -3246,6 +3248,9 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb, > } else if (ret > 0 && !overwrite && ext4_test_inode_state(inode, > EXT4_STATE_DIO_UNWRITTEN)) { > int err; > + err = ext4_es_convert_unwritten_extents(inode, offset, ret); > + if (err) > + ret = err; > /* > * for non AIO case, since the IO is already > * completed, we could do the conversion right here > diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c > index 0016fbc..66ea30e 100644 > --- a/fs/ext4/page-io.c > +++ b/fs/ext4/page-io.c > @@ -276,6 +276,13 @@ static void ext4_end_bio(struct bio *bio, int error) > error = 0; > bio_put(bio); > > + /* > + * We need to convert unwrittne extents in extent status tree before > + * end_page_writeback() is called. Otherwise, when dioread_nolock is > + * enabled, we will be likely to read stale data. > + */ > + inode = io_end->inode; > + ext4_es_convert_unwritten_extents(inode, io_end->offset, io_end->size); > for (i = 0; i < io_end->num_io_pages; i++) { > struct page *page = io_end->pages[i]->p_page; > struct buffer_head *bh, *head; > @@ -305,7 +312,6 @@ static void ext4_end_bio(struct bio *bio, int error) > put_io_page(io_end->pages[i]); > } > io_end->num_io_pages = 0; > - inode = io_end->inode; > > if (error) { > io_end->flag |= EXT4_IO_END_ERROR; > diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h > index f0734b3..d32e3d5 100644 > --- a/include/trace/events/ext4.h > +++ b/include/trace/events/ext4.h > @@ -2233,6 +2233,31 @@ TRACE_EVENT(ext4_es_lookup_extent_exit, > __entry->found ? __entry->status : 0) > ); > > +TRACE_EVENT(ext4_es_convert_unwritten_extents, > + TP_PROTO(struct inode *inode, loff_t offset, loff_t size), > + > + TP_ARGS(inode, offset, size), > + > + TP_STRUCT__entry( > + __field( dev_t, dev ) > + __field( ino_t, ino ) > + __field( loff_t, offset ) > + __field( loff_t, size ) > + ), > + > + TP_fast_assign( > + __entry->dev = inode->i_sb->s_dev; > + __entry->ino = inode->i_ino; > + __entry->offset = offset; > + __entry->size = size; > + ), > + > + TP_printk("dev %d,%d ino %lu convert unwritten extents [%llu/%llu", > + MAJOR(__entry->dev), MINOR(__entry->dev), > + (unsigned long) __entry->ino, > + __entry->offset, __entry->size) > +); > + > TRACE_EVENT(ext4_es_reclaim_extents_count, > TP_PROTO(struct super_block *sb, int nr_cached), > > -- > 1.7.12.rc2.18.g61b472e > -- 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