Re: [f2fs-dev] [PATCH v4 03/22] xfs: Use extent size granularity for iomap->io_block_size
On 12/06/2024 22:47, Darrick J. Wong wrote:
On Fri, Jun 07, 2024 at 02:39:00PM +, John Garry wrote:
Currently iomap->io_block_size is set to the i_blocksize() value for the
inode.
Expand the sub-fs block size zeroing to now cover RT extents, by calling
setting iomap->io_block_size as xfs_inode_alloc_unitsize().
In xfs_iomap_write_unwritten(), update the unwritten range fsb to cover
this extent granularity.
In xfs_file_dio_write(), handle a write which is not aligned to extent
size granularity as unaligned. Since the extent size granularity need not
be a power-of-2, handle this also.
Signed-off-by: John Garry
---
fs/xfs/xfs_file.c | 24 +++-
fs/xfs/xfs_inode.c | 17 +++--
fs/xfs/xfs_inode.h | 1 +
fs/xfs/xfs_iomap.c | 8 +++-
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index b240ea5241dc..24fe3c2e03da 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -601,7 +601,7 @@ xfs_file_dio_write_aligned(
}
/*
- * Handle block unaligned direct I/O writes
+ * Handle unaligned direct IO writes.
*
* In most cases direct I/O writes will be done holding IOLOCK_SHARED,
allowing
* them to be done in parallel with reads and other direct I/O writes.
However,
@@ -630,9 +630,9 @@ xfs_file_dio_write_unaligned(
ssize_t ret;
/*
-* Extending writes need exclusivity because of the sub-block zeroing
-* that the DIO code always does for partial tail blocks beyond EOF, so
-* don't even bother trying the fast path in this case.
+* Extending writes need exclusivity because of the sub-block/extent
+* zeroing that the DIO code always does for partial tail blocks
+* beyond EOF, so don't even bother trying the fast path in this case.
Hummm. So let's say the fsblock size is 4k, the rt extent size is 16k,
and you want to write bytes 8192-12287 of a file. Currently we'd use
xfs_file_dio_write_aligned for that, but now we'd use
xfs_file_dio_write_unaligned? Even though we don't need zeroing or any
of that stuff?
Right, this is something which I mentioned in response to the previous
patch.
I doubt whether we should only do this for atomic writes inodes, or also
RT and forcealign-only inodes.
I got the impression from Dave in review of the previous version of this
series that it should include RT and forcealign-only.
*/
if (iocb->ki_pos > isize || iocb->ki_pos + count >= isize) {
if (iocb->ki_flags & IOCB_NOWAIT)
@@ -698,11 +698,25 @@ xfs_file_dio_write(
struct xfs_inode*ip = XFS_I(file_inode(iocb->ki_filp));
struct xfs_buftarg *target = xfs_inode_buftarg(ip);
size_t count = iov_iter_count(from);
+ boolunaligned;
+ u64 unitsize;
/* direct I/O must be aligned to device logical sector size */
if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
return -EINVAL;
- if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
+
+ unitsize = xfs_inode_alloc_unitsize(ip);
+ if (!is_power_of_2(unitsize)) {
+ if (isaligned_64(iocb->ki_pos, unitsize) &&
+ isaligned_64(count, unitsize))
+ unaligned = false;
+ else
+ unaligned = true;
+ } else {
+ unaligned = (iocb->ki_pos | count) & (unitsize - 1);
+ }
Didn't I already write this?
It's from xfs_is_falloc_aligned(). Let's reuse that fully here. I did
look at doing that before, though...
+ if (unaligned)
if (!xfs_is_falloc_aligned(ip, iocb->ki_pos, count))
return xfs_file_dio_write_unaligned(ip, iocb, from);
return xfs_file_dio_write_aligned(ip, iocb, from);
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 58fb7a5062e1..93ad442f399b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -4264,15 +4264,20 @@ xfs_break_layouts(
return error;
}
-/* Returns the size of fundamental allocation unit for a file, in bytes. */
Don't delete the comment, it has useful return type information.
It wasn't deleted, it is still below.
/*
* Returns the size of fundamental allocation unit for a file, in
* fsblocks.
*/
unsigned int
-xfs_inode_alloc_unitsize(
+xfs_inode_alloc_unitsize_fsb(
struct xfs_inode*ip)
{
- unsigned intblocks = 1;
-
if (XFS_IS_REALTIME_INODE(ip))
- blocks = ip->i_mount->m_sb.sb_rextsize;
+ return ip->i_mount->m_sb.sb_rextsize;
+
+ return 1;
+}
- return XFS_FSB_TO_B(ip->i_mount, blocks);
+/* Returns the size of fundamental allocation unit for a file, in bytes. */
+unsigned int
+xfs_inode_alloc_unitsize(
+ struct xfs_inode*ip)
+{
+ return XFS_FSB_TO_B(ip->i_mount,
Re: [f2fs-dev] [PATCH v4 03/22] xfs: Use extent size granularity for iomap->io_block_size
On Fri, Jun 07, 2024 at 02:39:00PM +, John Garry wrote:
> Currently iomap->io_block_size is set to the i_blocksize() value for the
> inode.
>
> Expand the sub-fs block size zeroing to now cover RT extents, by calling
> setting iomap->io_block_size as xfs_inode_alloc_unitsize().
>
> In xfs_iomap_write_unwritten(), update the unwritten range fsb to cover
> this extent granularity.
>
> In xfs_file_dio_write(), handle a write which is not aligned to extent
> size granularity as unaligned. Since the extent size granularity need not
> be a power-of-2, handle this also.
>
> Signed-off-by: John Garry
> ---
> fs/xfs/xfs_file.c | 24 +++-
> fs/xfs/xfs_inode.c | 17 +++--
> fs/xfs/xfs_inode.h | 1 +
> fs/xfs/xfs_iomap.c | 8 +++-
> 4 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index b240ea5241dc..24fe3c2e03da 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -601,7 +601,7 @@ xfs_file_dio_write_aligned(
> }
>
> /*
> - * Handle block unaligned direct I/O writes
> + * Handle unaligned direct IO writes.
> *
> * In most cases direct I/O writes will be done holding IOLOCK_SHARED,
> allowing
> * them to be done in parallel with reads and other direct I/O writes.
> However,
> @@ -630,9 +630,9 @@ xfs_file_dio_write_unaligned(
> ssize_t ret;
>
> /*
> - * Extending writes need exclusivity because of the sub-block zeroing
> - * that the DIO code always does for partial tail blocks beyond EOF, so
> - * don't even bother trying the fast path in this case.
> + * Extending writes need exclusivity because of the sub-block/extent
> + * zeroing that the DIO code always does for partial tail blocks
> + * beyond EOF, so don't even bother trying the fast path in this case.
Hummm. So let's say the fsblock size is 4k, the rt extent size is 16k,
and you want to write bytes 8192-12287 of a file. Currently we'd use
xfs_file_dio_write_aligned for that, but now we'd use
xfs_file_dio_write_unaligned? Even though we don't need zeroing or any
of that stuff?
>*/
> if (iocb->ki_pos > isize || iocb->ki_pos + count >= isize) {
> if (iocb->ki_flags & IOCB_NOWAIT)
> @@ -698,11 +698,25 @@ xfs_file_dio_write(
> struct xfs_inode*ip = XFS_I(file_inode(iocb->ki_filp));
> struct xfs_buftarg *target = xfs_inode_buftarg(ip);
> size_t count = iov_iter_count(from);
> + boolunaligned;
> + u64 unitsize;
>
> /* direct I/O must be aligned to device logical sector size */
> if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
> return -EINVAL;
> - if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
> +
> + unitsize = xfs_inode_alloc_unitsize(ip);
> + if (!is_power_of_2(unitsize)) {
> + if (isaligned_64(iocb->ki_pos, unitsize) &&
> + isaligned_64(count, unitsize))
> + unaligned = false;
> + else
> + unaligned = true;
> + } else {
> + unaligned = (iocb->ki_pos | count) & (unitsize - 1);
> + }
Didn't I already write this?
> + if (unaligned)
if (!xfs_is_falloc_aligned(ip, iocb->ki_pos, count))
> return xfs_file_dio_write_unaligned(ip, iocb, from);
> return xfs_file_dio_write_aligned(ip, iocb, from);
> }
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 58fb7a5062e1..93ad442f399b 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -4264,15 +4264,20 @@ xfs_break_layouts(
> return error;
> }
>
> -/* Returns the size of fundamental allocation unit for a file, in bytes. */
Don't delete the comment, it has useful return type information.
/*
* Returns the size of fundamental allocation unit for a file, in
* fsblocks.
*/
> unsigned int
> -xfs_inode_alloc_unitsize(
> +xfs_inode_alloc_unitsize_fsb(
> struct xfs_inode*ip)
> {
> - unsigned intblocks = 1;
> -
> if (XFS_IS_REALTIME_INODE(ip))
> - blocks = ip->i_mount->m_sb.sb_rextsize;
> + return ip->i_mount->m_sb.sb_rextsize;
> +
> + return 1;
> +}
>
> - return XFS_FSB_TO_B(ip->i_mount, blocks);
> +/* Returns the size of fundamental allocation unit for a file, in bytes. */
> +unsigned int
> +xfs_inode_alloc_unitsize(
> + struct xfs_inode*ip)
> +{
> + return XFS_FSB_TO_B(ip->i_mount, xfs_inode_alloc_unitsize_fsb(ip));
> }
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 292b90b5f2ac..90d2fa837117 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -643,6 +643,7 @@ int xfs_inode_reload_unlinked(struct xfs_inode *ip);
> bool xfs_ifork_zapped(const struct xfs_inode *ip, int whichfork);
> void xfs_inode_count_blocks(struct xfs_trans *tp, struct xfs_inode *ip,
>
[f2fs-dev] [PATCH v4 03/22] xfs: Use extent size granularity for iomap->io_block_size
Currently iomap->io_block_size is set to the i_blocksize() value for the
inode.
Expand the sub-fs block size zeroing to now cover RT extents, by calling
setting iomap->io_block_size as xfs_inode_alloc_unitsize().
In xfs_iomap_write_unwritten(), update the unwritten range fsb to cover
this extent granularity.
In xfs_file_dio_write(), handle a write which is not aligned to extent
size granularity as unaligned. Since the extent size granularity need not
be a power-of-2, handle this also.
Signed-off-by: John Garry
---
fs/xfs/xfs_file.c | 24 +++-
fs/xfs/xfs_inode.c | 17 +++--
fs/xfs/xfs_inode.h | 1 +
fs/xfs/xfs_iomap.c | 8 +++-
4 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index b240ea5241dc..24fe3c2e03da 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -601,7 +601,7 @@ xfs_file_dio_write_aligned(
}
/*
- * Handle block unaligned direct I/O writes
+ * Handle unaligned direct IO writes.
*
* In most cases direct I/O writes will be done holding IOLOCK_SHARED, allowing
* them to be done in parallel with reads and other direct I/O writes.
However,
@@ -630,9 +630,9 @@ xfs_file_dio_write_unaligned(
ssize_t ret;
/*
-* Extending writes need exclusivity because of the sub-block zeroing
-* that the DIO code always does for partial tail blocks beyond EOF, so
-* don't even bother trying the fast path in this case.
+* Extending writes need exclusivity because of the sub-block/extent
+* zeroing that the DIO code always does for partial tail blocks
+* beyond EOF, so don't even bother trying the fast path in this case.
*/
if (iocb->ki_pos > isize || iocb->ki_pos + count >= isize) {
if (iocb->ki_flags & IOCB_NOWAIT)
@@ -698,11 +698,25 @@ xfs_file_dio_write(
struct xfs_inode*ip = XFS_I(file_inode(iocb->ki_filp));
struct xfs_buftarg *target = xfs_inode_buftarg(ip);
size_t count = iov_iter_count(from);
+ boolunaligned;
+ u64 unitsize;
/* direct I/O must be aligned to device logical sector size */
if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
return -EINVAL;
- if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
+
+ unitsize = xfs_inode_alloc_unitsize(ip);
+ if (!is_power_of_2(unitsize)) {
+ if (isaligned_64(iocb->ki_pos, unitsize) &&
+ isaligned_64(count, unitsize))
+ unaligned = false;
+ else
+ unaligned = true;
+ } else {
+ unaligned = (iocb->ki_pos | count) & (unitsize - 1);
+ }
+
+ if (unaligned)
return xfs_file_dio_write_unaligned(ip, iocb, from);
return xfs_file_dio_write_aligned(ip, iocb, from);
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 58fb7a5062e1..93ad442f399b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -4264,15 +4264,20 @@ xfs_break_layouts(
return error;
}
-/* Returns the size of fundamental allocation unit for a file, in bytes. */
unsigned int
-xfs_inode_alloc_unitsize(
+xfs_inode_alloc_unitsize_fsb(
struct xfs_inode*ip)
{
- unsigned intblocks = 1;
-
if (XFS_IS_REALTIME_INODE(ip))
- blocks = ip->i_mount->m_sb.sb_rextsize;
+ return ip->i_mount->m_sb.sb_rextsize;
+
+ return 1;
+}
- return XFS_FSB_TO_B(ip->i_mount, blocks);
+/* Returns the size of fundamental allocation unit for a file, in bytes. */
+unsigned int
+xfs_inode_alloc_unitsize(
+ struct xfs_inode*ip)
+{
+ return XFS_FSB_TO_B(ip->i_mount, xfs_inode_alloc_unitsize_fsb(ip));
}
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index 292b90b5f2ac..90d2fa837117 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -643,6 +643,7 @@ int xfs_inode_reload_unlinked(struct xfs_inode *ip);
bool xfs_ifork_zapped(const struct xfs_inode *ip, int whichfork);
void xfs_inode_count_blocks(struct xfs_trans *tp, struct xfs_inode *ip,
xfs_filblks_t *dblocks, xfs_filblks_t *rblocks);
+unsigned int xfs_inode_alloc_unitsize_fsb(struct xfs_inode *ip);
unsigned int xfs_inode_alloc_unitsize(struct xfs_inode *ip);
struct xfs_dir_update_params {
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index ecb4cae88248..fbe69f747e30 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -127,7 +127,7 @@ xfs_bmbt_to_iomap(
}
iomap->offset = XFS_FSB_TO_B(mp, imap->br_startoff);
iomap->length = XFS_FSB_TO_B(mp, imap->br_blockcount);
- iomap->io_block_size = i_blocksize(VFS_I(ip));
+ iomap->io_block_size = xfs_inode_alloc_unitsize(ip);
if (mapping_flags & IOMAP_DAX)
iomap->dax_dev = target->bt_daxdev;
