> -----Original Message-----
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
> Sent: 23 March 2024 12:39 AM
> To: Srikanth C S <srikanth.c.s@xxxxxxxxxx>
> Cc: linux-xfs@xxxxxxxxxxxxxxx; cem@xxxxxxxxxx; Rajesh
> Sivaramasubramaniom <rajesh.sivaramasubramaniom@xxxxxxxxxx>
> Subject: [External] : Re: [PATCH] xfs_repair: Dump both inode details in
> Phase 6 duplicate file check
>
> On Fri, Mar 22, 2024 at 04:45:12AM +0000, Srikanth C S wrote:
> > The current check for duplicate names only dumps the inode number of
> > the parent directory and the inode number of the actual inode in question.
> > But, the inode number of original inode is not dumped. This patch
> > dumps the original inode too.
> >
> > xfs_repair output before applying this patch Phase 6 - check inode
> > connectivity...
> > - traversing filesystem ...
> > entry "dup-name1" (ino 132) in dir 128 is a duplicate name, would junk
> > entry entry "dup-name1" (ino 133) in dir 128 is a duplicate name,
> > would junk entry
> >
> > After this patch
> > Phase 6 - check inode connectivity...
> > - traversing filesystem ...
> > entry "dup-name1" (ino 132) in dir 128 already points to ino 131,
> > would junk entry entry "dup-name1" (ino 133) in dir 128 already points
> > to ino 131, would junk entry
> >
> > The entry_junked() function takes in only 4 arguments. In order to
> > print the original inode number, modifying the function to take 5
> > parameters
> >
> > Signed-off-by: Srikanth C S <srikanth.c.s@xxxxxxxxxx>
> > ---
> > repair/phase6.c | 51
> > +++++++++++++++++++++++++++++--------------------
> > 1 file changed, 30 insertions(+), 21 deletions(-)
> >
> > diff --git a/repair/phase6.c b/repair/phase6.c index
> > 3870c5c9..148454d0 100644
> > --- a/repair/phase6.c
> > +++ b/repair/phase6.c
> > @@ -151,9 +151,10 @@ dir_read_buf(
> > }
> >
> > /*
> > - * Returns 0 if the name already exists (ie. a duplicate)
> > + * Returns inode number of original file if the name already exists
> > + * (ie. a duplicate)
> > */
> > -static int
> > +static xfs_ino_t
> > dir_hash_add(
> > struct xfs_mount *mp,
> > struct dir_hash_tab *hashtab,
> > @@ -166,7 +167,7 @@ dir_hash_add(
> > xfs_dahash_t hash = 0;
> > int byhash = 0;
> > struct dir_hash_ent *p;
> > - int dup;
> > + xfs_ino_t dup_inum;
> > short junk;
> > struct xfs_name xname;
> > int error;
> > @@ -176,7 +177,7 @@ dir_hash_add(
> > xname.type = ftype;
> >
> > junk = name[0] == '/';
> > - dup = 0;
> > + dup_inum = 0;
> >
> > if (!junk) {
> > hash = libxfs_dir2_hashname(mp, &xname); @@ -188,7
> +189,7 @@
> > dir_hash_add(
> > for (p = hashtab->byhash[byhash]; p; p = p->nextbyhash) {
> > if (p->hashval == hash && p->name.len == namelen)
> {
> > if (memcmp(p->name.name, name,
> namelen) == 0) {
> > - dup = 1;
> > + dup_inum = p->inum;
> > junk = 1;
> > break;
> > }
> > @@ -234,7 +235,7 @@ dir_hash_add(
> > p->name.name = p->namebuf;
> > p->name.len = namelen;
> > p->name.type = ftype;
> > - return !dup;
> > + return dup_inum;
> > }
> >
> > /* Mark an existing directory hashtable entry as junk. */ @@ -1173,9
> > +1174,13 @@ entry_junked(
> > const char *msg,
> > const char *iname,
> > xfs_ino_t ino1,
> > - xfs_ino_t ino2)
> > + xfs_ino_t ino2,
> > + xfs_ino_t ino3)
> > {
> > - do_warn(msg, iname, ino1, ino2);
> > + if(ino3)
>
> Hmm. Seeing as we have a symbol (NULLFSINO) for null values, perhaps this
> should be:
>
> if (ino3 != NULLFSINO)
> do_warn(msg, iname, ino1, ino2, ino3);
>
> Otherwise looks fine to me....
>
> --D
Yes, using the symbol NULLFSINO makes more sense. Will send out a V2.
Thanks,
Srikanth
>
> > + do_warn(msg, iname, ino1, ino2, ino3);
> > + else
> > + do_warn(msg, iname, ino1, ino2);
> > if (!no_modify)
> > do_warn(_("junking entry\n"));
> > else
> > @@ -1470,6 +1475,7 @@ longform_dir2_entry_check_data(
> > int i;
> > int ino_offset;
> > xfs_ino_t inum;
> > + xfs_ino_t dup_inum;
> > ino_tree_node_t *irec;
> > int junkit;
> > int lastfree;
> > @@ -1680,7 +1686,7 @@ longform_dir2_entry_check_data(
> > nbad++;
> > if (entry_junked(
> > _("entry \"%s\" in directory inode %" PRIu64 " points to non-existent
> inode %" PRIu64 ", "),
> > - fname, ip->i_ino, inum)) {
> > + fname, ip->i_ino, inum, 0)) {
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp, dep);
> > }
> > @@ -1697,7 +1703,7 @@ longform_dir2_entry_check_data(
> > nbad++;
> > if (entry_junked(
> > _("entry \"%s\" in directory inode %" PRIu64 " points to free inode
> %" PRIu64 ", "),
> > - fname, ip->i_ino, inum)) {
> > + fname, ip->i_ino, inum, 0)) {
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp, dep);
> > }
> > @@ -1715,7 +1721,7 @@ longform_dir2_entry_check_data(
> > nbad++;
> > if (entry_junked(
> > _("%s (ino %" PRIu64 ") in root (%" PRIu64 ") is not a directory, "),
> > - ORPHANAGE, inum, ip-
> >i_ino)) {
> > + ORPHANAGE, inum, ip-
> >i_ino, 0)) {
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp,
> dep);
> > }
> > @@ -1732,12 +1738,13 @@ longform_dir2_entry_check_data(
> > /*
> > * check for duplicate names in directory.
> > */
> > - if (!dir_hash_add(mp, hashtab, addr, inum, dep->namelen,
> > - dep->name, libxfs_dir2_data_get_ftype(mp,
> dep))) {
> > + dup_inum = dir_hash_add(mp, hashtab, addr, inum, dep-
> >namelen,
> > + dep->name, libxfs_dir2_data_get_ftype(mp,
> dep));
> > + if (dup_inum) {
> > nbad++;
> > if (entry_junked(
> > - _("entry \"%s\" (ino %" PRIu64 ") in dir %" PRIu64 " is a duplicate
> name, "),
> > - fname, inum, ip->i_ino)) {
> > + _("entry \"%s\" (ino %" PRIu64 ") in dir %" PRIu64 " already points to
> ino %" PRIu64 ", "),
> > + fname, inum, ip->i_ino, dup_inum)) {
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp, dep);
> > }
> > @@ -1768,7 +1775,7 @@ longform_dir2_entry_check_data(
> > nbad++;
> > if (entry_junked(
> > _("entry \"%s\" (ino %" PRIu64 ") in dir %" PRIu64 " is not in the the
> first block, "), fname,
> > - inum, ip->i_ino)) {
> > + inum, ip->i_ino, 0)) {
> > dir_hash_junkit(hashtab, addr);
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp,
> dep); @@ -1801,7 +1808,7 @@
> > longform_dir2_entry_check_data(
> > nbad++;
> > if (entry_junked(
> > _("entry \"%s\" in dir %" PRIu64 " is not the first entry, "),
> > - fname, inum, ip->i_ino)) {
> > + fname, inum, ip->i_ino, 0)) {
> > dir_hash_junkit(hashtab, addr);
> > dep->name[0] = '/';
> > libxfs_dir2_data_log_entry(&da, bp,
> dep); @@ -2456,6 +2463,7 @@
> > shortform_dir2_entry_check( {
> > xfs_ino_t lino;
> > xfs_ino_t parent;
> > + xfs_ino_t dup_inum;
> > struct xfs_dir2_sf_hdr *sfp;
> > struct xfs_dir2_sf_entry *sfep;
> > struct xfs_dir2_sf_entry *next_sfep; @@ -2639,13 +2647,14 @@
> > shortform_dir2_entry_check(
> > /*
> > * check for duplicate names in directory.
> > */
> > - if (!dir_hash_add(mp, hashtab, (xfs_dir2_dataptr_t)
> > + dup_inum = dir_hash_add(mp, hashtab, (xfs_dir2_dataptr_t)
> > (sfep - xfs_dir2_sf_firstentry(sfp)),
> > lino, sfep->namelen, sfep->name,
> > - libxfs_dir2_sf_get_ftype(mp, sfep))) {
> > + libxfs_dir2_sf_get_ftype(mp, sfep));
> > + if (dup_inum) {
> > do_warn(
> > -_("entry \"%s\" (ino %" PRIu64 ") in dir %" PRIu64 " is a duplicate name, "),
> > - fname, lino, ino);
> > +_("entry \"%s\" (ino %" PRIu64 ") in dir %" PRIu64 " already points to ino
> %" PRIu64 ", "),
> > + fname, lino, ino, dup_inum);
> > next_sfep = shortform_dir2_junk(mp, sfp, sfep, lino,
> > &max_size, &i,
> &bytes_deleted,
> > ino_dirty);
> > --
> > 2.25.1
> >
> >
