* [PATCH v1] libxfs: fix inode reservation space for removing transaction
@ 2022-08-02 3:18 Xiaole He
2022-08-09 20:44 ` Darrick J. Wong
0 siblings, 1 reply; 4+ messages in thread
From: Xiaole He @ 2022-08-02 3:18 UTC (permalink / raw)
To: linux-xfs; +Cc: hexiaole
From: hexiaole <hexiaole@kylinos.cn>
In 'libxfs/xfs_trans_resv.c', the comment for transaction of removing a
directory entry writes:
/* libxfs/xfs_trans_resv.c begin */
/*
* For removing a directory entry we can modify:
* the parent directory inode: inode size
* the removed inode: inode size
...
/* libxfs/xfs_trans_resv.c end */
There has 2 inode size of space to be reserverd, but the actual code
for inode reservation space writes:
/* libxfs/xfs_trans_resv.c begin */
/*
* For removing a directory entry we can modify:
* the parent directory inode: inode size
* the removed inode: inode size
...
xfs_calc_remove_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_add_reservation(mp) +
max((xfs_calc_inode_res(mp, 1) +
...
/* libxfs/xfs_trans_resv.c end */
There only count for 1 inode size to be reserved in
'xfs_calc_inode_res(mp, 1)', rather than 2.
Signed-off-by: hexiaole <hexiaole@kylinos.cn>
---
libxfs/xfs_trans_resv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libxfs/xfs_trans_resv.c b/libxfs/xfs_trans_resv.c
index d4a9f69e..797176d7 100644
--- a/libxfs/xfs_trans_resv.c
+++ b/libxfs/xfs_trans_resv.c
@@ -514,7 +514,7 @@ xfs_calc_remove_reservation(
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_add_reservation(mp) +
- max((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 2) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] libxfs: fix inode reservation space for removing transaction
2022-08-02 3:18 [PATCH v1] libxfs: fix inode reservation space for removing transaction Xiaole He
@ 2022-08-09 20:44 ` Darrick J. Wong
[not found] ` <3d62cb5d.a53.1828570fa65.Coremail.hexiaole1994@126.com>
0 siblings, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2022-08-09 20:44 UTC (permalink / raw)
To: Xiaole He; +Cc: linux-xfs, hexiaole
On Tue, Aug 02, 2022 at 11:18:06AM +0800, Xiaole He wrote:
> From: hexiaole <hexiaole@kylinos.cn>
>
> In 'libxfs/xfs_trans_resv.c', the comment for transaction of removing a
> directory entry writes:
>
> /* libxfs/xfs_trans_resv.c begin */
> /*
> * For removing a directory entry we can modify:
> * the parent directory inode: inode size
> * the removed inode: inode size
> ...
> /* libxfs/xfs_trans_resv.c end */
>
> There has 2 inode size of space to be reserverd, but the actual code
> for inode reservation space writes:
>
> /* libxfs/xfs_trans_resv.c begin */
> /*
> * For removing a directory entry we can modify:
> * the parent directory inode: inode size
> * the removed inode: inode size
> ...
> xfs_calc_remove_reservation(
> struct xfs_mount *mp)
> {
> return XFS_DQUOT_LOGRES(mp) +
> xfs_calc_iunlink_add_reservation(mp) +
> max((xfs_calc_inode_res(mp, 1) +
> ...
> /* libxfs/xfs_trans_resv.c end */
>
> There only count for 1 inode size to be reserved in
> 'xfs_calc_inode_res(mp, 1)', rather than 2.
The logic looks correct. Why is this patch against xfsprogs, though?
--D
> Signed-off-by: hexiaole <hexiaole@kylinos.cn>
> ---
> libxfs/xfs_trans_resv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libxfs/xfs_trans_resv.c b/libxfs/xfs_trans_resv.c
> index d4a9f69e..797176d7 100644
> --- a/libxfs/xfs_trans_resv.c
> +++ b/libxfs/xfs_trans_resv.c
> @@ -514,7 +514,7 @@ xfs_calc_remove_reservation(
> {
> return XFS_DQUOT_LOGRES(mp) +
> xfs_calc_iunlink_add_reservation(mp) +
> - max((xfs_calc_inode_res(mp, 1) +
> + max((xfs_calc_inode_res(mp, 2) +
> xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> XFS_FSB_TO_B(mp, 1))),
> (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH v1] libxfs: fix inode reservation space for removing transaction
[not found] ` <3d62cb5d.a53.1828570fa65.Coremail.hexiaole1994@126.com>
@ 2022-08-10 2:50 ` Darrick J. Wong
0 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2022-08-10 2:50 UTC (permalink / raw)
To: 何小乐; +Cc: linux-xfs, hexiaole
On Wed, Aug 10, 2022 at 09:49:10AM +0800, 何小乐 wrote:
> Thank you for review, Darrick.
> Yes, this problem(reserve 1 inode size rather than 2) exists in both
> the userspace utility 'xfsprogs' and kernel source 'xfs-linux'.
>
> The reason that sent patch against 'xfsprogs' only is that I'm not
> sure whether the patch is correct(whether there need reserve one more
> inode size for the removed inode or not) and hope the maintainers
Yep -- we do need to have enough space to log the parent directory inode
as well as the child that's being removed from the directory.
> helping to review for its correctness, if it's correct, I'm willing to
> send another patch against kernel source 'xfs-linux.
>
> So, Darric, you said 'The logic looks correct.' means the patch is
> correct, rather than the original logic that reserving 1 inode size
> for removing transaction is correct, right? Hopelly I did not
> misunderstand.
Yes, the patch looks correct to me. Please send us the kernel version,
so that we can land it in the kernel (and then xfsprogs). :)
--D
>
>
>
>
>
>
>
> At 2022-08-10 04:44:12, "Darrick J. Wong" <djwong@kernel.org> wrote:
> >On Tue, Aug 02, 2022 at 11:18:06AM +0800, Xiaole He wrote:
> >> From: hexiaole <hexiaole@kylinos.cn>
> >>
> >> In 'libxfs/xfs_trans_resv.c', the comment for transaction of removing a
> >> directory entry writes:
> >>
> >> /* libxfs/xfs_trans_resv.c begin */
> >> /*
> >> * For removing a directory entry we can modify:
> >> * the parent directory inode: inode size
> >> * the removed inode: inode size
> >> ...
> >> /* libxfs/xfs_trans_resv.c end */
> >>
> >> There has 2 inode size of space to be reserverd, but the actual code
> >> for inode reservation space writes:
> >>
> >> /* libxfs/xfs_trans_resv.c begin */
> >> /*
> >> * For removing a directory entry we can modify:
> >> * the parent directory inode: inode size
> >> * the removed inode: inode size
> >> ...
> >> xfs_calc_remove_reservation(
> >> struct xfs_mount *mp)
> >> {
> >> return XFS_DQUOT_LOGRES(mp) +
> >> xfs_calc_iunlink_add_reservation(mp) +
> >> max((xfs_calc_inode_res(mp, 1) +
> >> ...
> >> /* libxfs/xfs_trans_resv.c end */
> >>
> >> There only count for 1 inode size to be reserved in
> >> 'xfs_calc_inode_res(mp, 1)', rather than 2.
> >
> >The logic looks correct. Why is this patch against xfsprogs, though?
> >
> >--D
> >
> >> Signed-off-by: hexiaole <hexiaole@kylinos.cn>
> >> ---
> >> libxfs/xfs_trans_resv.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libxfs/xfs_trans_resv.c b/libxfs/xfs_trans_resv.c
> >> index d4a9f69e..797176d7 100644
> >> --- a/libxfs/xfs_trans_resv.c
> >> +++ b/libxfs/xfs_trans_resv.c
> >> @@ -514,7 +514,7 @@ xfs_calc_remove_reservation(
> >> {
> >> return XFS_DQUOT_LOGRES(mp) +
> >> xfs_calc_iunlink_add_reservation(mp) +
> >> - max((xfs_calc_inode_res(mp, 1) +
> >> + max((xfs_calc_inode_res(mp, 2) +
> >> xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
> >> XFS_FSB_TO_B(mp, 1))),
> >> (xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
> >> --
> >> 2.27.0
> >>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v1] libxfs: fix inode reservation space for removing transaction
@ 2022-08-10 6:29 Xiaole He
0 siblings, 0 replies; 4+ messages in thread
From: Xiaole He @ 2022-08-10 6:29 UTC (permalink / raw)
To: linux-xfs; +Cc: djwong, dchinner, chandan.babu, hexiaole
From: hexiaole <hexiaole@kylinos.cn>
In 'libxfs/xfs_trans_resv.c', the comment for transaction of removing a
directory entry writes:
/* libxfs/xfs_trans_resv.c begin */
/*
* For removing a directory entry we can modify:
* the parent directory inode: inode size
* the removed inode: inode size
...
/* libxfs/xfs_trans_resv.c end */
There has 2 inode size of space to be reserverd, but the actual code
for inode reservation space writes:
/* libxfs/xfs_trans_resv.c begin */
/*
* For removing a directory entry we can modify:
* the parent directory inode: inode size
* the removed inode: inode size
...
xfs_calc_remove_reservation(
struct xfs_mount *mp)
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_add_reservation(mp) +
max((xfs_calc_inode_res(mp, 1) +
...
/* libxfs/xfs_trans_resv.c end */
There only count for 1 inode size to be reserved in
'xfs_calc_inode_res(mp, 1)', rather than 2.
Signed-off-by: hexiaole <hexiaole@kylinos.cn>
---
fs/xfs/libxfs/xfs_trans_resv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_trans_resv.c b/fs/xfs/libxfs/xfs_trans_resv.c
index e9913c2c5a24..2c4ad6e4bb14 100644
--- a/fs/xfs/libxfs/xfs_trans_resv.c
+++ b/fs/xfs/libxfs/xfs_trans_resv.c
@@ -515,7 +515,7 @@ xfs_calc_remove_reservation(
{
return XFS_DQUOT_LOGRES(mp) +
xfs_calc_iunlink_add_reservation(mp) +
- max((xfs_calc_inode_res(mp, 1) +
+ max((xfs_calc_inode_res(mp, 2) +
xfs_calc_buf_res(XFS_DIROP_LOG_COUNT(mp),
XFS_FSB_TO_B(mp, 1))),
(xfs_calc_buf_res(4, mp->m_sb.sb_sectsize) +
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-08-10 6:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 3:18 [PATCH v1] libxfs: fix inode reservation space for removing transaction Xiaole He
2022-08-09 20:44 ` Darrick J. Wong
[not found] ` <3d62cb5d.a53.1828570fa65.Coremail.hexiaole1994@126.com>
2022-08-10 2:50 ` Darrick J. Wong
2022-08-10 6:29 Xiaole He
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.