* [PATCH 0/2] Set lost+found inode as used after its allocation
@ 2011-11-09 16:54 Carlos Maiolino
2011-11-09 16:54 ` [PATCH 1/2] xfs_repair: Add inline function to get avl tree node Carlos Maiolino
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Carlos Maiolino @ 2011-11-09 16:54 UTC (permalink / raw)
To: xfs; +Cc: Carlos Maiolino
This patch set ensure the inode allocated to the lost+found directory is
set as used in AVL tree if it needs to be created (in case no lost+found
directory is already created on the disk). Also, I added a new inline
function to get AVL tree node offset which currently is being calculated
whenever the offset is needed.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] xfs_repair: Add inline function to get avl tree node 2011-11-09 16:54 [PATCH 0/2] Set lost+found inode as used after its allocation Carlos Maiolino @ 2011-11-09 16:54 ` Carlos Maiolino 2011-11-09 16:54 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino 2011-11-10 11:11 ` [PATCH 0/2] Set lost+found inode as used after its allocation Christoph Hellwig 2 siblings, 0 replies; 9+ messages in thread From: Carlos Maiolino @ 2011-11-09 16:54 UTC (permalink / raw) To: xfs; +Cc: Carlos Maiolino Add get_inode_offset() inline function, which will return the offset of a specific node in the AVL tree avoiding the need to calculate the the offset each time it needs to be used. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@redhat.com> --- repair/incore.h | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/repair/incore.h b/repair/incore.h index ee0e86a..8e311c9 100644 --- a/repair/incore.h +++ b/repair/incore.h @@ -311,6 +311,12 @@ void get_inode_rec(struct xfs_mount *mp, xfs_agnumber_t agno, ino_tree_node_t *ino_rec); extern avltree_desc_t **inode_tree_ptrs; + +static inline int +get_inode_offset(struct xfs_mount *mp, xfs_ino_t ino, ino_tree_node_t *irec) +{ + return XFS_INO_TO_AGINO(mp, ino) - irec->ino_startnum; +} static inline ino_tree_node_t * findfirst_inode_rec(xfs_agnumber_t agno) { -- 1.7.6.4 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfs_repair: Properly set lost+found inode as used 2011-11-09 16:54 [PATCH 0/2] Set lost+found inode as used after its allocation Carlos Maiolino 2011-11-09 16:54 ` [PATCH 1/2] xfs_repair: Add inline function to get avl tree node Carlos Maiolino @ 2011-11-09 16:54 ` Carlos Maiolino 2011-11-14 18:09 ` Ben Myers 2011-11-10 11:11 ` [PATCH 0/2] Set lost+found inode as used after its allocation Christoph Hellwig 2 siblings, 1 reply; 9+ messages in thread From: Carlos Maiolino @ 2011-11-09 16:54 UTC (permalink / raw) To: xfs; +Cc: Carlos Maiolino This patch makes mk_orphanage() to properly set the inode link count of the recently allocated inode in the AVL tree, avoiding the lost+found directory to be bypass the link count check in phase7 and possibly leaving lost+found directory with a wrong link count. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Eric Sandeen <sandeen@redhat.com> --- repair/phase6.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-) diff --git a/repair/phase6.c b/repair/phase6.c index adad61d..d38e44f 100644 --- a/repair/phase6.c +++ b/repair/phase6.c @@ -823,6 +823,8 @@ mk_orphanage(xfs_mount_t *mp) xfs_inode_t *ip; xfs_inode_t *pip; xfs_fsblock_t first; + ino_tree_node_t *irec; + int ino_offset = 0; int i; int committed; int error; @@ -875,6 +877,19 @@ mk_orphanage(xfs_mount_t *mp) ORPHANAGE, error); } ip->i_d.di_nlink++; /* account for . */ + ino = ip->i_ino; + + irec = find_inode_rec(mp, + XFS_INO_TO_AGNO(mp, ino), + XFS_INO_TO_AGINO(mp, ino)); + ino_offset = get_inode_offset(mp, ino, irec); + + /* + * Mark the inode allocated to lost+found as used in the AVL tree + * so it is not skipped in phase 7 + */ + set_inode_used(irec, ino_offset); + add_inode_ref(irec, ino_offset); /* * now that we know the transaction will stay around, @@ -902,6 +917,7 @@ mk_orphanage(xfs_mount_t *mp) XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino)), 0); + libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE); libxfs_dir_init(tp, ip, pip); libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); @@ -912,9 +928,9 @@ mk_orphanage(xfs_mount_t *mp) ORPHANAGE, error); } - ino = ip->i_ino; libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC); + add_inode_reached(irec,ino_offset); return(ino); } -- 1.7.6.4 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs_repair: Properly set lost+found inode as used 2011-11-09 16:54 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino @ 2011-11-14 18:09 ` Ben Myers 0 siblings, 0 replies; 9+ messages in thread From: Ben Myers @ 2011-11-14 18:09 UTC (permalink / raw) To: Carlos Maiolino; +Cc: xfs Hey Carlos, On Wed, Nov 09, 2011 at 02:54:07PM -0200, Carlos Maiolino wrote: > This patch makes mk_orphanage() to properly set the inode link count of > the recently allocated inode in the AVL tree, avoiding the lost+found > directory to be bypass the link count check in phase7 and possibly leaving > lost+found directory with a wrong link count. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Eric Sandeen <sandeen@redhat.com> > --- > repair/phase6.c | 18 +++++++++++++++++- > 1 files changed, 17 insertions(+), 1 deletions(-) > > diff --git a/repair/phase6.c b/repair/phase6.c > index adad61d..d38e44f 100644 > --- a/repair/phase6.c > +++ b/repair/phase6.c > @@ -823,6 +823,8 @@ mk_orphanage(xfs_mount_t *mp) > xfs_inode_t *ip; > xfs_inode_t *pip; > xfs_fsblock_t first; > + ino_tree_node_t *irec; > + int ino_offset = 0; > int i; > int committed; > int error; > @@ -875,6 +877,19 @@ mk_orphanage(xfs_mount_t *mp) > ORPHANAGE, error); > } > ip->i_d.di_nlink++; /* account for . */ > + ino = ip->i_ino; > + > + irec = find_inode_rec(mp, > + XFS_INO_TO_AGNO(mp, ino), > + XFS_INO_TO_AGINO(mp, ino)); > + ino_offset = get_inode_offset(mp, ino, irec); > + > + /* > + * Mark the inode allocated to lost+found as used in the AVL tree > + * so it is not skipped in phase 7 > + */ > + set_inode_used(irec, ino_offset); > + add_inode_ref(irec, ino_offset); Both add_inode_ref()... > /* > * now that we know the transaction will stay around, > @@ -902,6 +917,7 @@ mk_orphanage(xfs_mount_t *mp) > XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino)), 0); > > > + > libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE); > libxfs_dir_init(tp, ip, pip); > libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); > @@ -912,9 +928,9 @@ mk_orphanage(xfs_mount_t *mp) > ORPHANAGE, error); > } > > - ino = ip->i_ino; > > libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC); > + add_inode_reached(irec,ino_offset); and add_inode_reached() call counted_nlink_inc. I have to admit to being an xfs_repair newbie, so you can tell me to take a hike if you want. Did you intend to bump up nlink twice? THat's not what I would have expected. THanks, Ben _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Set lost+found inode as used after its allocation 2011-11-09 16:54 [PATCH 0/2] Set lost+found inode as used after its allocation Carlos Maiolino 2011-11-09 16:54 ` [PATCH 1/2] xfs_repair: Add inline function to get avl tree node Carlos Maiolino 2011-11-09 16:54 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino @ 2011-11-10 11:11 ` Christoph Hellwig 2011-11-10 12:05 ` Carlos Maiolino 2 siblings, 1 reply; 9+ messages in thread From: Christoph Hellwig @ 2011-11-10 11:11 UTC (permalink / raw) To: Carlos Maiolino; +Cc: xfs Thanks a lot, I've put the patches in. I'd love to see followups to use the the get_inode_offset helper more often, and to add a testcase for the bug if possible. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Set lost+found inode as used after its allocation 2011-11-10 11:11 ` [PATCH 0/2] Set lost+found inode as used after its allocation Christoph Hellwig @ 2011-11-10 12:05 ` Carlos Maiolino 0 siblings, 0 replies; 9+ messages in thread From: Carlos Maiolino @ 2011-11-10 12:05 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs > I'd love to see followups to use the the get_inode_offset helper more > often, and to add a testcase for the bug if possible. Hi Chris, I added the xfstests test case creation to my todo list, I should have this implemented soon. -- --Carlos _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/2] Set lost+found inode as used after its allocation @ 2011-11-08 18:46 Carlos Maiolino 2011-11-08 18:46 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino 0 siblings, 1 reply; 9+ messages in thread From: Carlos Maiolino @ 2011-11-08 18:46 UTC (permalink / raw) To: xfs; +Cc: Carlos Maiolino This patch set ensure the inode allocated to the lost+found directory is set as used in AVL tree if it needs to be created (in case no lost+found directory is already created on the disk). Also, I added a new inline function to get AVL tree node offset which currently is being calculated whenever the offset is needed. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfs_repair: Properly set lost+found inode as used 2011-11-08 18:46 Carlos Maiolino @ 2011-11-08 18:46 ` Carlos Maiolino 2011-11-08 18:55 ` Eric Sandeen 2011-11-08 19:07 ` Christoph Hellwig 0 siblings, 2 replies; 9+ messages in thread From: Carlos Maiolino @ 2011-11-08 18:46 UTC (permalink / raw) To: xfs; +Cc: Carlos Maiolino This patch makes mk_orphanage() to properly set the inode link count of the recently allocated inode in the AVL tree, avoiding the lost+found directory to be bypass the link count check in phase7 and possibly leaving lost+found directory with a wrong link count. --- repair/phase6.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-) diff --git a/repair/phase6.c b/repair/phase6.c index adad61d..0e0e294 100644 --- a/repair/phase6.c +++ b/repair/phase6.c @@ -823,6 +823,8 @@ mk_orphanage(xfs_mount_t *mp) xfs_inode_t *ip; xfs_inode_t *pip; xfs_fsblock_t first; + ino_tree_node_t *irec; + int ino_offset = 0; int i; int committed; int error; @@ -875,6 +877,18 @@ mk_orphanage(xfs_mount_t *mp) ORPHANAGE, error); } ip->i_d.di_nlink++; /* account for . */ + ino = ip->i_ino; + + irec = find_inode_rec(mp, + XFS_INO_TO_AGNO(mp, ino), + XFS_INO_TO_AGINO(mp,ino)); + ino_offset = get_inode_offset(mp, ino, irec); + + /* Set the inode allocated to lost+found as used in the AVL + * tree, so it is not bypassed in phase 7 + */ + set_inode_used(irec,ino_offset); + add_inode_ref(irec,ino_offset); /* * now that we know the transaction will stay around, @@ -902,6 +916,8 @@ mk_orphanage(xfs_mount_t *mp) XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino)), 0); + add_inode_reached(irec,ino_offset); + libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE); libxfs_dir_init(tp, ip, pip); libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); @@ -912,7 +928,6 @@ mk_orphanage(xfs_mount_t *mp) ORPHANAGE, error); } - ino = ip->i_ino; libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC); -- 1.7.6.4 _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs_repair: Properly set lost+found inode as used 2011-11-08 18:46 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino @ 2011-11-08 18:55 ` Eric Sandeen 2011-11-08 19:07 ` Christoph Hellwig 1 sibling, 0 replies; 9+ messages in thread From: Eric Sandeen @ 2011-11-08 18:55 UTC (permalink / raw) To: Carlos Maiolino; +Cc: xfs On 11/8/11 12:46 PM, Carlos Maiolino wrote: > This patch makes mk_orphanage() to properly set the inode link count of > the recently allocated inode in the AVL tree, avoiding the lost+found > directory to be bypass the link count check in phase7 and possibly leaving > lost+found directory with a wrong link count. Looks pretty good to me. I think maybe add_inode_reached shouldn't be called until the transaction to do it is complete, though? Otherwise that might fail and we'd still have marked it as reached; I think that could be a problem. Thanks, -Eric > --- > repair/phase6.c | 17 ++++++++++++++++- > 1 files changed, 16 insertions(+), 1 deletions(-) > > diff --git a/repair/phase6.c b/repair/phase6.c > index adad61d..0e0e294 100644 > --- a/repair/phase6.c > +++ b/repair/phase6.c > @@ -823,6 +823,8 @@ mk_orphanage(xfs_mount_t *mp) > xfs_inode_t *ip; > xfs_inode_t *pip; > xfs_fsblock_t first; > + ino_tree_node_t *irec; > + int ino_offset = 0; > int i; > int committed; > int error; > @@ -875,6 +877,18 @@ mk_orphanage(xfs_mount_t *mp) > ORPHANAGE, error); > } > ip->i_d.di_nlink++; /* account for . */ > + ino = ip->i_ino; > + > + irec = find_inode_rec(mp, > + XFS_INO_TO_AGNO(mp, ino), > + XFS_INO_TO_AGINO(mp,ino)); > + ino_offset = get_inode_offset(mp, ino, irec); > + > + /* Set the inode allocated to lost+found as used in the AVL > + * tree, so it is not bypassed in phase 7 > + */ > + set_inode_used(irec,ino_offset); > + add_inode_ref(irec,ino_offset); > > /* > * now that we know the transaction will stay around, > @@ -902,6 +916,8 @@ mk_orphanage(xfs_mount_t *mp) > XFS_INO_TO_AGINO(mp, mp->m_sb.sb_rootino)), 0); > > > + add_inode_reached(irec,ino_offset); > + > libxfs_trans_log_inode(tp, pip, XFS_ILOG_CORE); > libxfs_dir_init(tp, ip, pip); > libxfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); > @@ -912,7 +928,6 @@ mk_orphanage(xfs_mount_t *mp) > ORPHANAGE, error); > } > > - ino = ip->i_ino; > > libxfs_trans_commit(tp, XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC); > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfs_repair: Properly set lost+found inode as used 2011-11-08 18:46 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino 2011-11-08 18:55 ` Eric Sandeen @ 2011-11-08 19:07 ` Christoph Hellwig 1 sibling, 0 replies; 9+ messages in thread From: Christoph Hellwig @ 2011-11-08 19:07 UTC (permalink / raw) To: Carlos Maiolino; +Cc: xfs On Tue, Nov 08, 2011 at 04:46:30PM -0200, Carlos Maiolino wrote: > This patch makes mk_orphanage() to properly set the inode link count of > the recently allocated inode in the AVL tree, avoiding the lost+found > directory to be bypass the link count check in phase7 and possibly leaving > lost+found directory with a wrong link count. Thanks a lot, this looks very good technically. Unfortunately I have a few style nipicks anyway: > @@ -875,6 +877,18 @@ mk_orphanage(xfs_mount_t *mp) > ORPHANAGE, error); > } > ip->i_d.di_nlink++; /* account for . */ > + ino = ip->i_ino; > + > + irec = find_inode_rec(mp, > + XFS_INO_TO_AGNO(mp, ino), > + XFS_INO_TO_AGINO(mp,ino)); Please always use whitespaces after the comma. > + ino_offset = get_inode_offset(mp, ino, irec); > + > + /* Set the inode allocated to lost+found as used in the AVL > + * tree, so it is not bypassed in phase 7 > + */ The canonical comment style would be: /* * Mark the inode allocated to lost+found as used in the AVL tree * so it is not skipped in phase 7. */ (also note the slight change in wording to make it easier to parse for me, but I'm no native speaker either) * > + set_inode_used(irec,ino_offset); > + add_inode_ref(irec,ino_offset); Same whitespace comment as above here. Feel free to add my: Reviewed-by: Christoph Hellwig <hch@lst.de> after these cosmetic fixups. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-11-14 18:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-09 16:54 [PATCH 0/2] Set lost+found inode as used after its allocation Carlos Maiolino 2011-11-09 16:54 ` [PATCH 1/2] xfs_repair: Add inline function to get avl tree node Carlos Maiolino 2011-11-09 16:54 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino 2011-11-14 18:09 ` Ben Myers 2011-11-10 11:11 ` [PATCH 0/2] Set lost+found inode as used after its allocation Christoph Hellwig 2011-11-10 12:05 ` Carlos Maiolino -- strict thread matches above, loose matches on Subject: below -- 2011-11-08 18:46 Carlos Maiolino 2011-11-08 18:46 ` [PATCH 2/2] xfs_repair: Properly set lost+found inode as used Carlos Maiolino 2011-11-08 18:55 ` Eric Sandeen 2011-11-08 19:07 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox