linux-ext4 - Re: [PATCH -v2] ext4: fake direct I/O mode for data=journal

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 17 Aug 2011 13:42:52 +0200
From:	Jan Kara <jack@...e.cz>
To:	Theodore Ts'o <tytso@....edu>
Cc:	Ext4 Developers List <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH -v2] ext4: fake direct I/O mode for data=journal

On Tue 16-08-11 15:32:25, Ted Tso wrote:
> Currently attempts to open a file with O_DIRECT in data=journal mode
> causes the open to fail with -EINVAL.  This makes it very hard to test
> data=journal mode.  So we will let the open succeed, but then always
> fall back to O_DSYNC buffered writes.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@....edu>
> ---
>  fs/ext4/file.c  |   28 +++++++++++++++++++++++++++-
>  fs/ext4/inode.c |    1 +
>  2 files changed, 28 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index e4095e9..566f33f 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -89,6 +89,24 @@ ext4_unaligned_aio(struct inode *inode, const struct iovec *iov,
>  	return 0;
>  }
>  
> +ssize_t
> +ext4_file_read(struct kiocb *iocb, const struct iovec *iov,
> +	       unsigned long nr_segs, loff_t pos)
> +{
> +	struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;
> +
> +	/*
> +	 * If O_DIRECT is set and we are doing data journalling we
> +	 * don't support O_DIRECT so force it off.
> +	 */
> +	if ((iocb->ki_filp->f_flags & O_DIRECT) &&
> +	    ext4_should_journal_data(inode))
> +		iocb->ki_filp->f_flags &= ~O_DIRECT;
> +
> +	return generic_file_aio_read(iocb, iov, nr_segs, pos);
> +}
> +
> +
  Two empty lines here?

>  static ssize_t
>  ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
>  		unsigned long nr_segs, loff_t pos)
> @@ -98,6 +116,14 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,
>  	int ret;
>  
>  	/*
> +	 * If O_DIRECT is set and we are doing data journalling we
> +	 * don't support O_DIRECT so force it off.
> +	 */
> +	if ((iocb->ki_filp->f_flags & O_DIRECT) &&
> +	    ext4_should_journal_data(inode))
> +		iocb->ki_filp->f_flags &= ~O_DIRECT;
> +
> +	/*
>  	 * If we have encountered a bitmap-format file, the size limit
>  	 * is smaller than s_maxbytes, which is for extent-mapped files.
>  	 */
> @@ -277,7 +303,7 @@ const struct file_operations ext4_file_operations = {
>  	.llseek		= ext4_llseek,
>  	.read		= do_sync_read,
>  	.write		= do_sync_write,
> -	.aio_read	= generic_file_aio_read,
> +	.aio_read	= ext4_file_read,
>  	.aio_write	= ext4_file_write,
>  	.unlocked_ioctl = ext4_ioctl,
>  #ifdef CONFIG_COMPAT
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 762e803..b7088e2 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -2921,6 +2921,7 @@ static const struct address_space_operations ext4_journalled_aops = {
>  	.bmap			= ext4_bmap,
>  	.invalidatepage		= ext4_invalidatepage,
>  	.releasepage		= ext4_releasepage,
> +	.direct_IO		= ext4_direct_IO,
>  	.is_partially_uptodate  = block_is_partially_uptodate,
>  	.error_remove_page	= generic_error_remove_page,
>  };
  Strictly speaking this is racy wrt. to fcntl() setting O_DIRECT bit.
Fixing it is actually quite simple - just provide ext4_noop_direct_IO
function that will just "return 0" and generic write code will fall
back to buffered IO automatically. We don't want to use the fallback all
the time since it incurs overhead of flushing and invalidating the mapping
so what you did above will catch the common case.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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