From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Matthew Bobrowski <mbobrowski@mbobrowski.org>
Cc: tytso@mit.edu, jack@suse.cz, adilger.kernel@dilger.ca,
linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
riteshh@linux.ibm.com
Subject: Re: [PATCH v7 06/11] ext4: introduce new callback for IOMAP_REPORT
Date: Tue, 5 Nov 2019 07:43:47 -0800 [thread overview]
Message-ID: <20191105154347.GB15203@magnolia> (raw)
In-Reply-To: <5c97a569e26ddb6696e3d3ac9fbde41317e029a0.1572949325.git.mbobrowski@mbobrowski.org>
On Tue, Nov 05, 2019 at 11:03:31PM +1100, Matthew Bobrowski wrote:
> As part of the ext4_iomap_begin() cleanups that precede this patch, we
> also split up the IOMAP_REPORT branch into a completely separate
> ->iomap_begin() callback named ext4_iomap_begin_report(). Again, the
> raionale for this change is to reduce the overall clutter within
> ext4_iomap_begin().
>
> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
> fs/ext4/ext4.h | 1 +
> fs/ext4/file.c | 6 ++-
> fs/ext4/inode.c | 134 +++++++++++++++++++++++++++++-------------------
> 3 files changed, 85 insertions(+), 56 deletions(-)
>
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 3616f1b0c987..5c6c4acea8b1 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -3388,6 +3388,7 @@ static inline void ext4_clear_io_unwritten_flag(ext4_io_end_t *io_end)
> }
>
> extern const struct iomap_ops ext4_iomap_ops;
> +extern const struct iomap_ops ext4_iomap_report_ops;
>
> static inline int ext4_buffer_uptodate(struct buffer_head *bh)
> {
> diff --git a/fs/ext4/file.c b/fs/ext4/file.c
> index 8d2bbcc2d813..ab75aee3e687 100644
> --- a/fs/ext4/file.c
> +++ b/fs/ext4/file.c
> @@ -494,12 +494,14 @@ loff_t ext4_llseek(struct file *file, loff_t offset, int whence)
> maxbytes, i_size_read(inode));
> case SEEK_HOLE:
> inode_lock_shared(inode);
> - offset = iomap_seek_hole(inode, offset, &ext4_iomap_ops);
> + offset = iomap_seek_hole(inode, offset,
> + &ext4_iomap_report_ops);
> inode_unlock_shared(inode);
> break;
> case SEEK_DATA:
> inode_lock_shared(inode);
> - offset = iomap_seek_data(inode, offset, &ext4_iomap_ops);
> + offset = iomap_seek_data(inode, offset,
> + &ext4_iomap_report_ops);
> inode_unlock_shared(inode);
> break;
> }
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index b540f2903faa..b5ba6767b276 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3553,74 +3553,32 @@ static int ext4_iomap_alloc(struct inode *inode, struct ext4_map_blocks *map,
> static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
> unsigned flags, struct iomap *iomap, struct iomap *srcmap)
> {
> - unsigned int blkbits = inode->i_blkbits;
> - unsigned long first_block, last_block;
> - struct ext4_map_blocks map;
> - bool delalloc = false;
> int ret;
> + struct ext4_map_blocks map;
> + u8 blkbits = inode->i_blkbits;
>
> if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
> return -EINVAL;
> - first_block = offset >> blkbits;
> - last_block = min_t(loff_t, (offset + length - 1) >> blkbits,
> - EXT4_MAX_LOGICAL_BLOCK);
> -
> - if (flags & IOMAP_REPORT) {
> - if (ext4_has_inline_data(inode)) {
> - ret = ext4_inline_data_iomap(inode, iomap);
> - if (ret != -EAGAIN) {
> - if (ret == 0 && offset >= iomap->length)
> - ret = -ENOENT;
> - return ret;
> - }
> - }
> - } else {
> - if (WARN_ON_ONCE(ext4_has_inline_data(inode)))
> - return -ERANGE;
> - }
>
> - map.m_lblk = first_block;
> - map.m_len = last_block - first_block + 1;
> -
> - if (flags & IOMAP_REPORT) {
> - ret = ext4_map_blocks(NULL, inode, &map, 0);
> - if (ret < 0)
> - return ret;
> -
> - if (ret == 0) {
> - ext4_lblk_t end = map.m_lblk + map.m_len - 1;
> - struct extent_status es;
> -
> - ext4_es_find_extent_range(inode, &ext4_es_is_delayed,
> - map.m_lblk, end, &es);
> + if (WARN_ON_ONCE(ext4_has_inline_data(inode)))
> + return -ERANGE;
>
> - if (!es.es_len || es.es_lblk > end) {
> - /* entire range is a hole */
> - } else if (es.es_lblk > map.m_lblk) {
> - /* range starts with a hole */
> - map.m_len = es.es_lblk - map.m_lblk;
> - } else {
> - ext4_lblk_t offs = 0;
> + /*
> + * Calculate the first and last logical blocks respectively.
> + */
> + map.m_lblk = offset >> blkbits;
> + map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
> + EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
>
> - if (es.es_lblk < map.m_lblk)
> - offs = map.m_lblk - es.es_lblk;
> - map.m_lblk = es.es_lblk + offs;
> - map.m_len = es.es_len - offs;
> - delalloc = true;
> - }
> - }
> - } else if (flags & IOMAP_WRITE) {
> + if (flags & IOMAP_WRITE)
> ret = ext4_iomap_alloc(inode, &map, flags);
FWIW you could even split non-buffered read and write into separate iomap
ops and avoid this split... but that's a cleanup that can wait until
after the main series lands.
> - } else {
> + else
> ret = ext4_map_blocks(NULL, inode, &map, 0);
> - }
>
> if (ret < 0)
> return ret;
>
> ext4_set_iomap(inode, iomap, &map, offset, length);
> - if (delalloc && iomap->type == IOMAP_HOLE)
> - iomap->type = IOMAP_DELALLOC;
>
> return 0;
> }
> @@ -3682,6 +3640,74 @@ const struct iomap_ops ext4_iomap_ops = {
> .iomap_end = ext4_iomap_end,
> };
>
> +static bool ext4_iomap_is_delalloc(struct inode *inode,
> + struct ext4_map_blocks *map)
> +{
> + struct extent_status es;
> + ext4_lblk_t offset = 0, end = map->m_lblk + map->m_len - 1;
> +
> + ext4_es_find_extent_range(inode, &ext4_es_is_delayed,
> + map->m_lblk, end, &es);
> +
> + if (!es.es_len || es.es_lblk > end)
> + return false;
> +
> + if (es.es_lblk > map->m_lblk) {
> + map->m_len = es.es_lblk - map->m_lblk;
> + return false;
> + }
> +
> + offset = map->m_lblk - es.es_lblk;
> + map->m_len = es.es_len - offset;
> +
> + return true;
> +}
> +
> +static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
> + loff_t length, unsigned int flags,
> + struct iomap *iomap, struct iomap *srcmap)
> +{
> + int ret;
> + bool delalloc = false;
> + struct ext4_map_blocks map;
> + u8 blkbits = inode->i_blkbits;
> +
> + if ((offset >> blkbits) > EXT4_MAX_LOGICAL_BLOCK)
> + return -EINVAL;
> +
> + if (ext4_has_inline_data(inode)) {
> + ret = ext4_inline_data_iomap(inode, iomap);
> + if (ret != -EAGAIN) {
> + if (ret == 0 && offset >= iomap->length)
> + ret = -ENOENT;
> + return ret;
> + }
> + }
> +
> + /*
> + * Calculate the first and last logical block respectively.
> + */
> + map.m_lblk = offset >> blkbits;
> + map.m_len = min_t(loff_t, (offset + length - 1) >> blkbits,
> + EXT4_MAX_LOGICAL_BLOCK) - map.m_lblk + 1;
> +
> + ret = ext4_map_blocks(NULL, inode, &map, 0);
> + if (ret < 0)
> + return ret;
> + if (ret == 0)
> + delalloc = ext4_iomap_is_delalloc(inode, &map);
If you can tell that a mapping is delalloc from @inode and @map, how
about pushing the ext4_iomap_is_delalloc call into ext4_set_iomap?
Oh, humm, the _is_delalloc function isn't a predicate after all; it
modifies @map. Urrk.
--D
> +
> + ext4_set_iomap(inode, iomap, &map, offset, length);
> + if (delalloc && iomap->type == IOMAP_HOLE)
> + iomap->type = IOMAP_DELALLOC;
> +
> + return 0;
> +}
> +
> +const struct iomap_ops ext4_iomap_report_ops = {
> + .iomap_begin = ext4_iomap_begin_report,
> +};
> +
> static int ext4_end_io_dio(struct kiocb *iocb, loff_t offset,
> ssize_t size, void *private)
> {
> --
> 2.20.1
>
prev parent reply other threads:[~2019-11-05 15:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-05 11:58 [PATCH v7 00/11] ext4: port direct I/O to iomap infrastructure Matthew Bobrowski
2019-11-05 11:58 ` [PATCH v7 01/11] ext4: reorder map.m_flags checks within ext4_iomap_begin() Matthew Bobrowski
2019-11-05 11:59 ` [PATCH v7 02/11] ext4: update direct I/O read lock pattern for IOCB_NOWAIT Matthew Bobrowski
2019-11-05 11:59 ` [PATCH v7 03/11] ext4: iomap that extends beyond EOF should be marked dirty Matthew Bobrowski
2019-11-05 11:59 ` [PATCH v7 04/11] ext4: move set iomap routines into a separate helper ext4_set_iomap() Matthew Bobrowski
2019-11-05 12:00 ` [PATCH v7 05/11] ext4: split IOMAP_WRITE branch in ext4_iomap_begin() into helper Matthew Bobrowski
2019-11-05 12:01 ` [PATCH v7 07/11] ext4: introduce direct I/O read using iomap infrastructure Matthew Bobrowski
2019-11-05 12:01 ` [PATCH v7 08/11] ext4: move inode extension/truncate code out from ->iomap_end() callback Matthew Bobrowski
2019-11-05 15:49 ` Darrick J. Wong
2019-11-05 21:10 ` Matthew Bobrowski
2019-11-05 12:02 ` [PATCH v7 09/11] ext4: move inode extension check out from ext4_iomap_alloc() Matthew Bobrowski
2019-11-05 12:02 ` [PATCH v7 10/11] ext4: update ext4_sync_file() to not use __generic_file_fsync() Matthew Bobrowski
2019-11-05 12:02 ` [PATCH v7 11/11] ext4: introduce direct I/O write using iomap infrastructure Matthew Bobrowski
2019-11-05 13:59 ` Jan Kara
2019-11-05 20:32 ` Matthew Bobrowski
2019-11-05 20:53 ` Theodore Y. Ts'o
2019-11-05 21:00 ` Matthew Bobrowski
2019-11-06 0:59 ` Theodore Y. Ts'o
2019-11-05 16:28 ` Theodore Y. Ts'o
2019-11-05 20:57 ` Matthew Bobrowski
2019-11-05 12:03 ` [PATCH v7 06/11] ext4: introduce new callback for IOMAP_REPORT Matthew Bobrowski
2019-11-05 15:43 ` Darrick J. Wong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191105154347.GB15203@magnolia \
--to=darrick.wong@oracle.com \
--cc=adilger.kernel@dilger.ca \
--cc=jack@suse.cz \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=mbobrowski@mbobrowski.org \
--cc=riteshh@linux.ibm.com \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.