From: "Darrick J. Wong" <djwong@kernel.org>
To: Srikanth C S <srikanth.c.s@oracle.com>
Cc: linux-xfs@vger.kernel.org, cem@kernel.org,
rajesh.sivaramasubramaniom@oracle.com
Subject: Re: [PATCH v2] xfs_repair: Dump both inode details in Phase 6 duplicate file check
Date: Mon, 25 Mar 2024 08:57:08 -0700 [thread overview]
Message-ID: <20240325155708.GC6390@frogsfrogsfrogs> (raw)
In-Reply-To: <20240325063443.2398800-1-srikanth.c.s@oracle.com>
On Mon, Mar 25, 2024 at 06:34:43AM +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@oracle.com>
Looks good to me now,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> repair/phase6.c | 51 +++++++++++++++++++++++++++++--------------------
> 1 file changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 3870c5c9..3d5af436 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 = NULLFSINO;
>
> 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 != NULLFSINO)
> + 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, NULLFSINO)) {
> 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, NULLFSINO)) {
> 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, NULLFSINO)) {
> 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 != NULLFSINO) {
> 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, NULLFSINO)) {
> 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, NULLFSINO)) {
> 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 != NULLFSINO) {
> 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
>
>
prev parent reply other threads:[~2024-03-25 15:57 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-25 6:34 [PATCH v2] xfs_repair: Dump both inode details in Phase 6 duplicate file check Srikanth C S
2024-03-25 15:57 ` 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=20240325155708.GC6390@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=rajesh.sivaramasubramaniom@oracle.com \
--cc=srikanth.c.s@oracle.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).