On Mon, Jun 05, 2017 at 08:03:59PM -0400, Theodore Ts'o wrote:
> There were a number of bugs in ext4_try_to_write_inline_data() and the
> ext4_convert_inline_data_to_extent() function (which was only used by
> ext4_try_to_write_inline_data).
>
> For ext4_convert_inline_data_to_extent():
>
> * It didn't handle the dioread_nolock case correctly
> * It didn't convert the extent tree entry from unwritten to written.
> * It didn't correctly handle racing DIO reads
> * It didn't handle data=journal case correctly -- it doesn't follow
> the block modification correctly by failing to call
> ext4_handle_dirty_metadata() on the data block.
>
> We fix this by eliminating ext4_convert_inline_data_to_extent()
> completely, and use reg_convert_inline_data_nolock() since it has been
> fixed to be Completely Correct (tm). :-)
>
Is ext4_da_convert_inline_data_to_extent() broken too?
> /*
> * Try to write data in the inode.
> * If the inode has inline data, check whether the new write can be
> @@ -662,13 +553,19 @@ int ext4_try_to_write_inline_data(struct address_space *mapping,
> struct page *page;
> struct ext4_iloc iloc;
>
> - if (pos + len > ext4_get_max_inline_size(inode))
> - goto convert;
> -
> ret = ext4_get_inode_loc(inode, &iloc);
> if (ret)
> return ret;
>
> + page = grab_cache_page_write_begin(mapping, 0, flags);
> + if (!page) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
Likewise, doesn't the page lock rank below transaction start? Also this jumps
to 'out' which looks at 'handle' before it's been initialized.
Eric
