[PATCH] cifs: Show reason why autodisabling serverino support

[PATCH] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

Before calling cifs_autodisable_serverino() function, show reason why it
has to be called.

This change allows to debug issues why cifs.ko decide to turn off server
inode number support and hence disable support for detection of hardlinks.

Signed-off-by: Pali Rohár <pali@kernel.org>
Cc: stable@vger.kernel.org
---
 fs/smb/client/connect.c   | 2 ++
 fs/smb/client/dfs_cache.c | 2 ++
 fs/smb/client/inode.c     | 3 +++
 fs/smb/client/readdir.c   | 3 +++
 4 files changed, 10 insertions(+)

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 6bf04d9a5491..e2dbf7eaf32a 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -3908,6 +3908,8 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
 	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
 	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
 	 */
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
+		cifs_dbg(VFS, "Reconnecting to different server, inode numbers won't match anymore\n");
 	cifs_autodisable_serverino(cifs_sb);
 	/*
 	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
index 4dada26d56b5..bb5bf9f45557 100644
--- a/fs/smb/client/dfs_cache.c
+++ b/fs/smb/client/dfs_cache.c
@@ -1289,6 +1289,8 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
 	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
 	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
 	 */
+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
+		cifs_dbg(VFS, "Reconnecting to different server, inode numbers won't match anymore\n");
 	cifs_autodisable_serverino(cifs_sb);
 	/*
 	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index cd06598eacbd..c6da25520f29 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1077,6 +1077,7 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
 			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
 		else {
 			fattr->cf_uniqueid = iunique(sb, ROOT_I);
+			cifs_dbg(VFS, "Cannot retrieve inode number for %s: %d\n", full_path, rc);
 			cifs_autodisable_serverino(cifs_sb);
 		}
 		return;
@@ -1530,6 +1531,7 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
 			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
 
 			if (inode_has_hashed_dentries(inode)) {
+				cifs_dbg(VFS, "Inode number collision detected\n");
 				cifs_autodisable_serverino(CIFS_SB(sb));
 				iput(inode);
 				fattr->cf_uniqueid = iunique(sb, ROOT_I);
@@ -1597,6 +1599,7 @@ struct inode *cifs_root_iget(struct super_block *sb)
 	if (!rc) {
 		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
 			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
+			cifs_dbg(VFS, "Cannot retrieve attributes for junction point %s: %d\n", path, rc);
 			cifs_autodisable_serverino(cifs_sb);
 		}
 		inode = cifs_iget(sb, &fattr);
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 787d6bcb5d1d..1235b5bf9814 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -413,6 +413,7 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
 		cifsFile->invalidHandle = false;
 	} else if ((rc == -EOPNOTSUPP) &&
 		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
+		cifs_dbg(VFS, "Cannot retrieve inode number for entries in dir %s: %d\n", full_path, rc);
 		cifs_autodisable_serverino(cifs_sb);
 		goto ffirst_retry;
 	}
@@ -1007,6 +1008,8 @@ static int cifs_filldir(char *find_entry, struct file *file,
 		fattr.cf_uniqueid = de.ino;
 	} else {
 		fattr.cf_uniqueid = iunique(sb, ROOT_I);
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
+			cifs_dbg(VFS, "Cannot retrieve inode number for dir entry %.*s\n", name.len, name.name);
 		cifs_autodisable_serverino(cifs_sb);
 	}
 
-- 
2.20.1

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> Before calling cifs_autodisable_serverino() function, show reason why it
> has to be called.
>
> This change allows to debug issues why cifs.ko decide to turn off server
> inode number support and hence disable support for detection of hardlinks.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  fs/smb/client/connect.c   | 2 ++
>  fs/smb/client/dfs_cache.c | 2 ++
>  fs/smb/client/inode.c     | 3 +++
>  fs/smb/client/readdir.c   | 3 +++
>  4 files changed, 10 insertions(+)

NACK.

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Steve French]

Since this could flood logs (e.g. in some DFS cases), probably better
to do these via the usual dynamic trace points (and can document a
simple "trace-cmd -e smb3_disable_serverino" script to avoid risk of
flooding logs.    cifsFYI is an alternative but the world has moved to
the dynamic tracing (eBPF etc.)

On Sun, Jun 8, 2025 at 3:57 PM Paulo Alcantara <pc@manguebit.org> wrote:
>
> Pali Rohár <pali@kernel.org> writes:
>
> > Before calling cifs_autodisable_serverino() function, show reason why it
> > has to be called.
> >
> > This change allows to debug issues why cifs.ko decide to turn off server
> > inode number support and hence disable support for detection of hardlinks.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Cc: stable@vger.kernel.org
> > ---
> >  fs/smb/client/connect.c   | 2 ++
> >  fs/smb/client/dfs_cache.c | 2 ++
> >  fs/smb/client/inode.c     | 3 +++
> >  fs/smb/client/readdir.c   | 3 +++
> >  4 files changed, 10 insertions(+)
>
> NACK.
>


-- 
Thanks,

Steve

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

cifs_autodisable_serverino() is already printing at the VFS level
information that it is disabling the serverino. It is one time thing as
once it is disabled, it is not printing it second time. So it does not
flood logs.

In this change I have just extended this existing logging to print also reason.

On Sunday 08 June 2025 22:40:21 Steve French wrote:
> Since this could flood logs (e.g. in some DFS cases), probably better
> to do these via the usual dynamic trace points (and can document a
> simple "trace-cmd -e smb3_disable_serverino" script to avoid risk of
> flooding logs.    cifsFYI is an alternative but the world has moved to
> the dynamic tracing (eBPF etc.)
> 
> On Sun, Jun 8, 2025 at 3:57 PM Paulo Alcantara <pc@manguebit.org> wrote:
> >
> > Pali Rohár <pali@kernel.org> writes:
> >
> > > Before calling cifs_autodisable_serverino() function, show reason why it
> > > has to be called.
> > >
> > > This change allows to debug issues why cifs.ko decide to turn off server
> > > inode number support and hence disable support for detection of hardlinks.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  fs/smb/client/connect.c   | 2 ++
> > >  fs/smb/client/dfs_cache.c | 2 ++
> > >  fs/smb/client/inode.c     | 3 +++
> > >  fs/smb/client/readdir.c   | 3 +++
> > >  4 files changed, 10 insertions(+)
> >
> > NACK.
> >
> 
> 
> -- 
> Thanks,
> 
> Steve

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Tom Talpey]

The message text is pretty cryptic: "inode numbers won't match anymore".
Can this be stated in a more actionable way? The primary consumer of
the log is the sysadmin, after all.

At a minimum, stating that hardlink detection won't work, as noted in
the commit. Perhaps more helpfully, state that it's not a client issue.

Tom.

On 6/9/2025 3:36 AM, Pali Rohár wrote:
> cifs_autodisable_serverino() is already printing at the VFS level
> information that it is disabling the serverino. It is one time thing as
> once it is disabled, it is not printing it second time. So it does not
> flood logs.
> 
> In this change I have just extended this existing logging to print also reason.
> 
> On Sunday 08 June 2025 22:40:21 Steve French wrote:
>> Since this could flood logs (e.g. in some DFS cases), probably better
>> to do these via the usual dynamic trace points (and can document a
>> simple "trace-cmd -e smb3_disable_serverino" script to avoid risk of
>> flooding logs.    cifsFYI is an alternative but the world has moved to
>> the dynamic tracing (eBPF etc.)
>>
>> On Sun, Jun 8, 2025 at 3:57 PM Paulo Alcantara <pc@manguebit.org> wrote:
>>>
>>> Pali Rohár <pali@kernel.org> writes:
>>>
>>>> Before calling cifs_autodisable_serverino() function, show reason why it
>>>> has to be called.
>>>>
>>>> This change allows to debug issues why cifs.ko decide to turn off server
>>>> inode number support and hence disable support for detection of hardlinks.
>>>>
>>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>>> Cc: stable@vger.kernel.org
>>>> ---
>>>>   fs/smb/client/connect.c   | 2 ++
>>>>   fs/smb/client/dfs_cache.c | 2 ++
>>>>   fs/smb/client/inode.c     | 3 +++
>>>>   fs/smb/client/readdir.c   | 3 +++
>>>>   4 files changed, 10 insertions(+)
>>>
>>> NACK.
>>>
>>
>>
>> -- 
>> Thanks,
>>
>> Steve
> 
> 

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

For sure I can improve also existing text message. But Tom already sent
NACK so I'm not sure if I should continue with improvement or discard it.

On Tuesday 10 June 2025 11:36:30 Tom Talpey wrote:
> The message text is pretty cryptic: "inode numbers won't match anymore".
> Can this be stated in a more actionable way? The primary consumer of
> the log is the sysadmin, after all.
> 
> At a minimum, stating that hardlink detection won't work, as noted in
> the commit. Perhaps more helpfully, state that it's not a client issue.
> 
> Tom.
> 
> On 6/9/2025 3:36 AM, Pali Rohár wrote:
> > cifs_autodisable_serverino() is already printing at the VFS level
> > information that it is disabling the serverino. It is one time thing as
> > once it is disabled, it is not printing it second time. So it does not
> > flood logs.
> > 
> > In this change I have just extended this existing logging to print also reason.
> > 
> > On Sunday 08 June 2025 22:40:21 Steve French wrote:
> > > Since this could flood logs (e.g. in some DFS cases), probably better
> > > to do these via the usual dynamic trace points (and can document a
> > > simple "trace-cmd -e smb3_disable_serverino" script to avoid risk of
> > > flooding logs.    cifsFYI is an alternative but the world has moved to
> > > the dynamic tracing (eBPF etc.)
> > > 
> > > On Sun, Jun 8, 2025 at 3:57 PM Paulo Alcantara <pc@manguebit.org> wrote:
> > > > 
> > > > Pali Rohár <pali@kernel.org> writes:
> > > > 
> > > > > Before calling cifs_autodisable_serverino() function, show reason why it
> > > > > has to be called.
> > > > > 
> > > > > This change allows to debug issues why cifs.ko decide to turn off server
> > > > > inode number support and hence disable support for detection of hardlinks.
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Cc: stable@vger.kernel.org
> > > > > ---
> > > > >   fs/smb/client/connect.c   | 2 ++
> > > > >   fs/smb/client/dfs_cache.c | 2 ++
> > > > >   fs/smb/client/inode.c     | 3 +++
> > > > >   fs/smb/client/readdir.c   | 3 +++
> > > > >   4 files changed, 10 insertions(+)
> > > > 
> > > > NACK.
> > > > 
> > > 
> > > 
> > > -- 
> > > Thanks,
> > > 
> > > Steve
> > 
> > 
> 

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> For sure I can improve also existing text message. But Tom already sent
> NACK so I'm not sure if I should continue with improvement or discard it.

Oh, feel free to continue as I'm not the CIFS maintainer, so my NACK
doesn't matter much.

Go ahead with Steve's suggestion to make those dynamic tracepoints.
These VFS messages will simply flood dmesg when reconnecting,
automounting DFS links or surrogate reparse points -- note that they
expire after 10 minutes, so next access will print the VFS message
again.

Re: [PATCH] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

On Tuesday 10 June 2025 14:10:00 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > For sure I can improve also existing text message. But Tom already sent
> > NACK so I'm not sure if I should continue with improvement or discard it.
> 
> Oh, feel free to continue as I'm not the CIFS maintainer, so my NACK
> doesn't matter much.
> 
> Go ahead with Steve's suggestion to make those dynamic tracepoints.
> These VFS messages will simply flood dmesg when reconnecting,
> automounting DFS links or surrogate reparse points -- note that they
> expire after 10 minutes, so next access will print the VFS message
> again.

Ok, I will try to improve it and I will start with the text messages.
Tom is right that that currently those text messages are missing
important details.

[PATCH v2] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

Extend cifs_autodisable_serverino() function to print also text message why
the function was called.

The text message is printed just once for mount then autodisabling
serverino support. Once the serverino support is disabled for mount it will
not be re-enabled. So those text messages do not cause flooding logs.

This change allows to debug issues why cifs.ko decide to turn off server
inode number support and hence disable support for detection of hardlinks.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Paulo and Tom, could you check if this change is better now for you?
It should address problems with logs flooding and also information about
harlinks (it is already printed as can be seen also in this diff).
I would like to get your ACK, so I'm trying to improve it.
---
 fs/smb/client/cifsproto.h | 2 +-
 fs/smb/client/connect.c   | 2 +-
 fs/smb/client/dfs_cache.c | 2 +-
 fs/smb/client/inode.c     | 6 +++---
 fs/smb/client/misc.c      | 6 +++++-
 fs/smb/client/readdir.c   | 4 ++--
 6 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index d550662b4e72..07a67c8c37ce 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -586,9 +586,9 @@ extern int cifs_do_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
 			   const struct nls_table *nls_codepage, int remap);
 extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
 			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
 #endif /* CIFS_ALLOW_INSECURE_LEGACY */
-extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
+extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc);
 extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
 extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
 			      struct cifs_sb_info *cifs_sb,
 			      struct cifs_fattr *fattr,
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 6bf04d9a5491..819721dfd5bb 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -3907,9 +3907,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
 	/*
 	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
 	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
 	 */
-	cifs_autodisable_serverino(cifs_sb);
+	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
 	/*
 	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
 	 * that have different prefix paths.
 	 */
diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
index 4dada26d56b5..c3fe85c31e2b 100644
--- a/fs/smb/client/dfs_cache.c
+++ b/fs/smb/client/dfs_cache.c
@@ -1288,9 +1288,9 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
 	/*
 	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
 	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
 	 */
-	cifs_autodisable_serverino(cifs_sb);
+	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
 	/*
 	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
 	 * that have different prefix paths.
 	 */
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index cd06598eacbd..b1c6e3986278 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1076,9 +1076,9 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
 		if (*inode)
 			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
 		else {
 			fattr->cf_uniqueid = iunique(sb, ROOT_I);
-			cifs_autodisable_serverino(cifs_sb);
+			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via get_srv_inum", rc);
 		}
 		return;
 	}
 
@@ -1529,9 +1529,9 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
 		if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) {
 			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
 
 			if (inode_has_hashed_dentries(inode)) {
-				cifs_autodisable_serverino(CIFS_SB(sb));
+				cifs_autodisable_serverino(CIFS_SB(sb), "Inode number collision detected", 0);
 				iput(inode);
 				fattr->cf_uniqueid = iunique(sb, ROOT_I);
 				goto retry_iget5_locked;
 			}
@@ -1596,9 +1596,9 @@ struct inode *cifs_root_iget(struct super_block *sb)
 iget_root:
 	if (!rc) {
 		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
 			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
-			cifs_autodisable_serverino(cifs_sb);
+			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve attributes for junction point", rc);
 		}
 		inode = cifs_iget(sb, &fattr);
 	}
 
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index e77017f47084..409277883e8a 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -551,9 +551,9 @@ dump_smb(void *buf, int smb_buf_length)
 		       smb_buf_length, true);
 }
 
 void
-cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
+cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc)
 {
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
 		struct cifs_tcon *tcon = NULL;
 
@@ -561,8 +561,12 @@ cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
 			tcon = cifs_sb_master_tcon(cifs_sb);
 
 		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
 		cifs_sb->mnt_cifs_serverino_autodisabled = true;
+		if (rc)
+			cifs_dbg(VFS, "%s: %d\n", reason, rc);
+		else
+			cifs_dbg(VFS, "%s\n", reason);
 		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s\n",
 			 tcon ? tcon->tree_name : "new server");
 		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS)\n");
 		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
index 787d6bcb5d1d..06e90921f751 100644
--- a/fs/smb/client/readdir.c
+++ b/fs/smb/client/readdir.c
@@ -412,9 +412,9 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
 	if (rc == 0) {
 		cifsFile->invalidHandle = false;
 	} else if ((rc == -EOPNOTSUPP) &&
 		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
-		cifs_autodisable_serverino(cifs_sb);
+		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via query_dir_first", rc);
 		goto ffirst_retry;
 	}
 error_exit:
 	cifs_put_tlink(tlink);
@@ -1006,9 +1006,9 @@ static int cifs_filldir(char *find_entry, struct file *file,
 	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
 		fattr.cf_uniqueid = de.ino;
 	} else {
 		fattr.cf_uniqueid = iunique(sb, ROOT_I);
-		cifs_autodisable_serverino(cifs_sb);
+		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number", 0);
 	}
 
 	if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MF_SYMLINKS) &&
 	    couldbe_mf_symlink(&fattr))
-- 
2.20.1

Re: [PATCH v2] cifs: Show reason why autodisabling serverino support

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> Extend cifs_autodisable_serverino() function to print also text message why
> the function was called.
>
> The text message is printed just once for mount then autodisabling
> serverino support. Once the serverino support is disabled for mount it will
> not be re-enabled. So those text messages do not cause flooding logs.
>
> This change allows to debug issues why cifs.ko decide to turn off server
> inode number support and hence disable support for detection of hardlinks.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Paulo and Tom, could you check if this change is better now for you?
> It should address problems with logs flooding and also information about
> harlinks (it is already printed as can be seen also in this diff).
> I would like to get your ACK, so I'm trying to improve it.
> ---
>  fs/smb/client/cifsproto.h | 2 +-
>  fs/smb/client/connect.c   | 2 +-
>  fs/smb/client/dfs_cache.c | 2 +-
>  fs/smb/client/inode.c     | 6 +++---
>  fs/smb/client/misc.c      | 6 +++++-
>  fs/smb/client/readdir.c   | 4 ++--
>  6 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> index d550662b4e72..07a67c8c37ce 100644
> --- a/fs/smb/client/cifsproto.h
> +++ b/fs/smb/client/cifsproto.h
> @@ -586,9 +586,9 @@ extern int cifs_do_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
>  			   const struct nls_table *nls_codepage, int remap);
>  extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
>  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
>  #endif /* CIFS_ALLOW_INSECURE_LEGACY */
> -extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
> +extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc);
>  extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
>  extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>  			      struct cifs_sb_info *cifs_sb,
>  			      struct cifs_fattr *fattr,
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 6bf04d9a5491..819721dfd5bb 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -3907,9 +3907,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
>  	/*
>  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
>  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
>  	 */
> -	cifs_autodisable_serverino(cifs_sb);
> +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);

We are mounting an DFS share, not reconnecting.  The message is
misleading.

>  	/*
>  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
>  	 * that have different prefix paths.
>  	 */
> diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
> index 4dada26d56b5..c3fe85c31e2b 100644
> --- a/fs/smb/client/dfs_cache.c
> +++ b/fs/smb/client/dfs_cache.c
> @@ -1288,9 +1288,9 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
>  	/*
>  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
>  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
>  	 */
> -	cifs_autodisable_serverino(cifs_sb);
> +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);

Ditto.

>  	/*
>  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
>  	 * that have different prefix paths.
>  	 */
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index cd06598eacbd..b1c6e3986278 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -1076,9 +1076,9 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
>  		if (*inode)
>  			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
>  		else {
>  			fattr->cf_uniqueid = iunique(sb, ROOT_I);
> -			cifs_autodisable_serverino(cifs_sb);
> +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via get_srv_inum", rc);

Looks good.

>  		}
>  		return;
>  	}
>  
> @@ -1529,9 +1529,9 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
>  		if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) {
>  			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
>  
>  			if (inode_has_hashed_dentries(inode)) {
> -				cifs_autodisable_serverino(CIFS_SB(sb));
> +				cifs_autodisable_serverino(CIFS_SB(sb), "Inode number collision detected", 0);

Looks good.

>  				iput(inode);
>  				fattr->cf_uniqueid = iunique(sb, ROOT_I);
>  				goto retry_iget5_locked;
>  			}
> @@ -1596,9 +1596,9 @@ struct inode *cifs_root_iget(struct super_block *sb)
>  iget_root:
>  	if (!rc) {
>  		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
>  			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
> -			cifs_autodisable_serverino(cifs_sb);
> +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve attributes for junction point", rc);

This has nothing to do with not being able to retrieve attributes.  It
is simply disabling 'serverino' to prevent inode collisions with
surrogate reparse points (automounts).  This should also printed with
FYI.

>  		}
>  		inode = cifs_iget(sb, &fattr);
>  	}
>  
> diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> index e77017f47084..409277883e8a 100644
> --- a/fs/smb/client/misc.c
> +++ b/fs/smb/client/misc.c
> @@ -551,9 +551,9 @@ dump_smb(void *buf, int smb_buf_length)
>  		       smb_buf_length, true);
>  }
>  
>  void
> -cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> +cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc)
>  {
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
>  		struct cifs_tcon *tcon = NULL;
>  
> @@ -561,8 +561,12 @@ cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
>  			tcon = cifs_sb_master_tcon(cifs_sb);
>  
>  		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
>  		cifs_sb->mnt_cifs_serverino_autodisabled = true;
> +		if (rc)
> +			cifs_dbg(VFS, "%s: %d\n", reason, rc);
> +		else
> +			cifs_dbg(VFS, "%s\n", reason);
>  		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s\n",
>  			 tcon ? tcon->tree_name : "new server");
>  		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS)\n");
>  		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
> diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> index 787d6bcb5d1d..06e90921f751 100644
> --- a/fs/smb/client/readdir.c
> +++ b/fs/smb/client/readdir.c
> @@ -412,9 +412,9 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
>  	if (rc == 0) {
>  		cifsFile->invalidHandle = false;
>  	} else if ((rc == -EOPNOTSUPP) &&
>  		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> -		cifs_autodisable_serverino(cifs_sb);
> +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via query_dir_first", rc);

Looks good.

>  		goto ffirst_retry;
>  	}
>  error_exit:
>  	cifs_put_tlink(tlink);
> @@ -1006,9 +1006,9 @@ static int cifs_filldir(char *find_entry, struct file *file,
>  	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
>  		fattr.cf_uniqueid = de.ino;
>  	} else {
>  		fattr.cf_uniqueid = iunique(sb, ROOT_I);
> -		cifs_autodisable_serverino(cifs_sb);
> +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number", 0);

Perhaps also mention which function it wasn't able to retrieve inode
number from like above?

Re: [PATCH v2] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

On Tuesday 17 June 2025 19:23:15 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > Extend cifs_autodisable_serverino() function to print also text message why
> > the function was called.
> >
> > The text message is printed just once for mount then autodisabling
> > serverino support. Once the serverino support is disabled for mount it will
> > not be re-enabled. So those text messages do not cause flooding logs.
> >
> > This change allows to debug issues why cifs.ko decide to turn off server
> > inode number support and hence disable support for detection of hardlinks.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > Paulo and Tom, could you check if this change is better now for you?
> > It should address problems with logs flooding and also information about
> > harlinks (it is already printed as can be seen also in this diff).
> > I would like to get your ACK, so I'm trying to improve it.
> > ---
> >  fs/smb/client/cifsproto.h | 2 +-
> >  fs/smb/client/connect.c   | 2 +-
> >  fs/smb/client/dfs_cache.c | 2 +-
> >  fs/smb/client/inode.c     | 6 +++---
> >  fs/smb/client/misc.c      | 6 +++++-
> >  fs/smb/client/readdir.c   | 4 ++--
> >  6 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> > index d550662b4e72..07a67c8c37ce 100644
> > --- a/fs/smb/client/cifsproto.h
> > +++ b/fs/smb/client/cifsproto.h
> > @@ -586,9 +586,9 @@ extern int cifs_do_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
> >  			   const struct nls_table *nls_codepage, int remap);
> >  extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
> >  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
> >  #endif /* CIFS_ALLOW_INSECURE_LEGACY */
> > -extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
> > +extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc);
> >  extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
> >  extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> >  			      struct cifs_sb_info *cifs_sb,
> >  			      struct cifs_fattr *fattr,
> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> > index 6bf04d9a5491..819721dfd5bb 100644
> > --- a/fs/smb/client/connect.c
> > +++ b/fs/smb/client/connect.c
> > @@ -3907,9 +3907,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
> >  	/*
> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
> >  	 */
> > -	cifs_autodisable_serverino(cifs_sb);
> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
> 
> We are mounting an DFS share, not reconnecting.  The message is
> misleading.

I mostly copied the comment above the cifs_autodisable_serverino() call.
Does it mean that the existing comment about reconnecting is wrong too?

> >  	/*
> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
> >  	 * that have different prefix paths.
> >  	 */
> > diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
> > index 4dada26d56b5..c3fe85c31e2b 100644
> > --- a/fs/smb/client/dfs_cache.c
> > +++ b/fs/smb/client/dfs_cache.c
> > @@ -1288,9 +1288,9 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
> >  	/*
> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
> >  	 */
> > -	cifs_autodisable_serverino(cifs_sb);
> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
> 
> Ditto.
> 
> >  	/*
> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
> >  	 * that have different prefix paths.
> >  	 */
> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> > index cd06598eacbd..b1c6e3986278 100644
> > --- a/fs/smb/client/inode.c
> > +++ b/fs/smb/client/inode.c
> > @@ -1076,9 +1076,9 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
> >  		if (*inode)
> >  			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
> >  		else {
> >  			fattr->cf_uniqueid = iunique(sb, ROOT_I);
> > -			cifs_autodisable_serverino(cifs_sb);
> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via get_srv_inum", rc);
> 
> Looks good.
> 
> >  		}
> >  		return;
> >  	}
> >  
> > @@ -1529,9 +1529,9 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
> >  		if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) {
> >  			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
> >  
> >  			if (inode_has_hashed_dentries(inode)) {
> > -				cifs_autodisable_serverino(CIFS_SB(sb));
> > +				cifs_autodisable_serverino(CIFS_SB(sb), "Inode number collision detected", 0);
> 
> Looks good.
> 
> >  				iput(inode);
> >  				fattr->cf_uniqueid = iunique(sb, ROOT_I);
> >  				goto retry_iget5_locked;
> >  			}
> > @@ -1596,9 +1596,9 @@ struct inode *cifs_root_iget(struct super_block *sb)
> >  iget_root:
> >  	if (!rc) {
> >  		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
> >  			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
> > -			cifs_autodisable_serverino(cifs_sb);
> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve attributes for junction point", rc);
> 
> This has nothing to do with not being able to retrieve attributes.  It
> is simply disabling 'serverino' to prevent inode collisions with
> surrogate reparse points (automounts).  This should also printed with
> FYI.

Ok. So then I misunderstood the code around. Do you know when exactly
can this case happen? And it is really a problem? Because name surrogate
reparse point already creates a new mount hierarchy for which is
generated new st_dev major and minor numbers and hence inode collisions
should not happen (they do not share st_dev anymore).

> >  		}
> >  		inode = cifs_iget(sb, &fattr);
> >  	}
> >  
> > diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> > index e77017f47084..409277883e8a 100644
> > --- a/fs/smb/client/misc.c
> > +++ b/fs/smb/client/misc.c
> > @@ -551,9 +551,9 @@ dump_smb(void *buf, int smb_buf_length)
> >  		       smb_buf_length, true);
> >  }
> >  
> >  void
> > -cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> > +cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc)
> >  {
> >  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
> >  		struct cifs_tcon *tcon = NULL;
> >  
> > @@ -561,8 +561,12 @@ cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> >  			tcon = cifs_sb_master_tcon(cifs_sb);
> >  
> >  		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
> >  		cifs_sb->mnt_cifs_serverino_autodisabled = true;
> > +		if (rc)
> > +			cifs_dbg(VFS, "%s: %d\n", reason, rc);
> > +		else
> > +			cifs_dbg(VFS, "%s\n", reason);
> >  		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s\n",
> >  			 tcon ? tcon->tree_name : "new server");
> >  		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS)\n");
> >  		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
> > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> > index 787d6bcb5d1d..06e90921f751 100644
> > --- a/fs/smb/client/readdir.c
> > +++ b/fs/smb/client/readdir.c
> > @@ -412,9 +412,9 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
> >  	if (rc == 0) {
> >  		cifsFile->invalidHandle = false;
> >  	} else if ((rc == -EOPNOTSUPP) &&
> >  		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> > -		cifs_autodisable_serverino(cifs_sb);
> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via query_dir_first", rc);
> 
> Looks good.
> 
> >  		goto ffirst_retry;
> >  	}
> >  error_exit:
> >  	cifs_put_tlink(tlink);
> > @@ -1006,9 +1006,9 @@ static int cifs_filldir(char *find_entry, struct file *file,
> >  	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> >  		fattr.cf_uniqueid = de.ino;
> >  	} else {
> >  		fattr.cf_uniqueid = iunique(sb, ROOT_I);
> > -		cifs_autodisable_serverino(cifs_sb);
> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number", 0);
> 
> Perhaps also mention which function it wasn't able to retrieve inode
> number from like above?

I quickly look at this code around and I was not able to figure out what
is that function which was not able to retrieve inode. So I did not
write it into the message. Do you know, or could you figure out what is
that function / callback?

Re: [PATCH v2] cifs: Show reason why autodisabling serverino support

[Paulo Alcantara]

Pali Rohár <pali@kernel.org> writes:

> On Tuesday 17 June 2025 19:23:15 Paulo Alcantara wrote:
>> Pali Rohár <pali@kernel.org> writes:
>> 
>> > Extend cifs_autodisable_serverino() function to print also text message why
>> > the function was called.
>> >
>> > The text message is printed just once for mount then autodisabling
>> > serverino support. Once the serverino support is disabled for mount it will
>> > not be re-enabled. So those text messages do not cause flooding logs.
>> >
>> > This change allows to debug issues why cifs.ko decide to turn off server
>> > inode number support and hence disable support for detection of hardlinks.
>> >
>> > Signed-off-by: Pali Rohár <pali@kernel.org>
>> > ---
>> > Paulo and Tom, could you check if this change is better now for you?
>> > It should address problems with logs flooding and also information about
>> > harlinks (it is already printed as can be seen also in this diff).
>> > I would like to get your ACK, so I'm trying to improve it.
>> > ---
>> >  fs/smb/client/cifsproto.h | 2 +-
>> >  fs/smb/client/connect.c   | 2 +-
>> >  fs/smb/client/dfs_cache.c | 2 +-
>> >  fs/smb/client/inode.c     | 6 +++---
>> >  fs/smb/client/misc.c      | 6 +++++-
>> >  fs/smb/client/readdir.c   | 4 ++--
>> >  6 files changed, 13 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
>> > index d550662b4e72..07a67c8c37ce 100644
>> > --- a/fs/smb/client/cifsproto.h
>> > +++ b/fs/smb/client/cifsproto.h
>> > @@ -586,9 +586,9 @@ extern int cifs_do_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
>> >  			   const struct nls_table *nls_codepage, int remap);
>> >  extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
>> >  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
>> >  #endif /* CIFS_ALLOW_INSECURE_LEGACY */
>> > -extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
>> > +extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc);
>> >  extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
>> >  extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
>> >  			      struct cifs_sb_info *cifs_sb,
>> >  			      struct cifs_fattr *fattr,
>> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
>> > index 6bf04d9a5491..819721dfd5bb 100644
>> > --- a/fs/smb/client/connect.c
>> > +++ b/fs/smb/client/connect.c
>> > @@ -3907,9 +3907,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
>> >  	/*
>> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
>> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
>> >  	 */
>> > -	cifs_autodisable_serverino(cifs_sb);
>> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
>> 
>> We are mounting an DFS share, not reconnecting.  The message is
>> misleading.
>
> I mostly copied the comment above the cifs_autodisable_serverino() call.
> Does it mean that the existing comment about reconnecting is wrong too?

The comment is trying to say why it disabled 'serverino'.  DFS failover
may potentially connect to a different server and share, hence the inode
numbers will no longer be valid.  The function is also called
cifs_mount().

>> >  	/*
>> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
>> >  	 * that have different prefix paths.
>> >  	 */
>> > diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
>> > index 4dada26d56b5..c3fe85c31e2b 100644
>> > --- a/fs/smb/client/dfs_cache.c
>> > +++ b/fs/smb/client/dfs_cache.c
>> > @@ -1288,9 +1288,9 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
>> >  	/*
>> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
>> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
>> >  	 */
>> > -	cifs_autodisable_serverino(cifs_sb);
>> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
>> 
>> Ditto.
>> 
>> >  	/*
>> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
>> >  	 * that have different prefix paths.
>> >  	 */
>> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
>> > index cd06598eacbd..b1c6e3986278 100644
>> > --- a/fs/smb/client/inode.c
>> > +++ b/fs/smb/client/inode.c
>> > @@ -1076,9 +1076,9 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
>> >  		if (*inode)
>> >  			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
>> >  		else {
>> >  			fattr->cf_uniqueid = iunique(sb, ROOT_I);
>> > -			cifs_autodisable_serverino(cifs_sb);
>> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via get_srv_inum", rc);
>> 
>> Looks good.
>> 
>> >  		}
>> >  		return;
>> >  	}
>> >  
>> > @@ -1529,9 +1529,9 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
>> >  		if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) {
>> >  			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
>> >  
>> >  			if (inode_has_hashed_dentries(inode)) {
>> > -				cifs_autodisable_serverino(CIFS_SB(sb));
>> > +				cifs_autodisable_serverino(CIFS_SB(sb), "Inode number collision detected", 0);
>> 
>> Looks good.
>> 
>> >  				iput(inode);
>> >  				fattr->cf_uniqueid = iunique(sb, ROOT_I);
>> >  				goto retry_iget5_locked;
>> >  			}
>> > @@ -1596,9 +1596,9 @@ struct inode *cifs_root_iget(struct super_block *sb)
>> >  iget_root:
>> >  	if (!rc) {
>> >  		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
>> >  			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
>> > -			cifs_autodisable_serverino(cifs_sb);
>> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve attributes for junction point", rc);
>> 
>> This has nothing to do with not being able to retrieve attributes.  It
>> is simply disabling 'serverino' to prevent inode collisions with
>> surrogate reparse points (automounts).  This should also printed with
>> FYI.
>
> Ok. So then I misunderstood the code around. Do you know when exactly
> can this case happen? And it is really a problem? Because name surrogate
> reparse point already creates a new mount hierarchy for which is
> generated new st_dev major and minor numbers and hence inode collisions
> should not happen (they do not share st_dev anymore).

There was a bug report of someone having inode collisions in a share
that had a reparse mount point, so the server was returning duplicate
inode numbers for files inside that share.  That is why we set those
directories as automounts and then disable 'serverino' only for them, so
the parent mount can still rely on the inode numbers from the server and
having hardlinks working.

Note that disabling 'serverino' means that the client won't trust the
inode numbers from the server and it will generate its own inode
numbers.  I don't understand why st_dev is relevant here.

>
>> >  		}
>> >  		inode = cifs_iget(sb, &fattr);
>> >  	}
>> >  
>> > diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
>> > index e77017f47084..409277883e8a 100644
>> > --- a/fs/smb/client/misc.c
>> > +++ b/fs/smb/client/misc.c
>> > @@ -551,9 +551,9 @@ dump_smb(void *buf, int smb_buf_length)
>> >  		       smb_buf_length, true);
>> >  }
>> >  
>> >  void
>> > -cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
>> > +cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc)
>> >  {
>> >  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
>> >  		struct cifs_tcon *tcon = NULL;
>> >  
>> > @@ -561,8 +561,12 @@ cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
>> >  			tcon = cifs_sb_master_tcon(cifs_sb);
>> >  
>> >  		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
>> >  		cifs_sb->mnt_cifs_serverino_autodisabled = true;
>> > +		if (rc)
>> > +			cifs_dbg(VFS, "%s: %d\n", reason, rc);
>> > +		else
>> > +			cifs_dbg(VFS, "%s\n", reason);
>> >  		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s\n",
>> >  			 tcon ? tcon->tree_name : "new server");
>> >  		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS)\n");
>> >  		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
>> > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
>> > index 787d6bcb5d1d..06e90921f751 100644
>> > --- a/fs/smb/client/readdir.c
>> > +++ b/fs/smb/client/readdir.c
>> > @@ -412,9 +412,9 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
>> >  	if (rc == 0) {
>> >  		cifsFile->invalidHandle = false;
>> >  	} else if ((rc == -EOPNOTSUPP) &&
>> >  		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
>> > -		cifs_autodisable_serverino(cifs_sb);
>> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via query_dir_first", rc);
>> 
>> Looks good.
>> 
>> >  		goto ffirst_retry;
>> >  	}
>> >  error_exit:
>> >  	cifs_put_tlink(tlink);
>> > @@ -1006,9 +1006,9 @@ static int cifs_filldir(char *find_entry, struct file *file,
>> >  	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
>> >  		fattr.cf_uniqueid = de.ino;
>> >  	} else {
>> >  		fattr.cf_uniqueid = iunique(sb, ROOT_I);
>> > -		cifs_autodisable_serverino(cifs_sb);
>> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number", 0);
>> 
>> Perhaps also mention which function it wasn't able to retrieve inode
>> number from like above?
>
> I quickly look at this code around and I was not able to figure out what
> is that function which was not able to retrieve inode. So I did not
> write it into the message. Do you know, or could you figure out what is
> that function / callback?

"Cannot retrieve inode number from readdir"

Re: [PATCH v2] cifs: Show reason why autodisabling serverino support

[Pali Rohár]

On Tuesday 17 June 2025 20:48:00 Paulo Alcantara wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > On Tuesday 17 June 2025 19:23:15 Paulo Alcantara wrote:
> >> Pali Rohár <pali@kernel.org> writes:
> >> 
> >> > Extend cifs_autodisable_serverino() function to print also text message why
> >> > the function was called.
> >> >
> >> > The text message is printed just once for mount then autodisabling
> >> > serverino support. Once the serverino support is disabled for mount it will
> >> > not be re-enabled. So those text messages do not cause flooding logs.
> >> >
> >> > This change allows to debug issues why cifs.ko decide to turn off server
> >> > inode number support and hence disable support for detection of hardlinks.
> >> >
> >> > Signed-off-by: Pali Rohár <pali@kernel.org>
> >> > ---
> >> > Paulo and Tom, could you check if this change is better now for you?
> >> > It should address problems with logs flooding and also information about
> >> > harlinks (it is already printed as can be seen also in this diff).
> >> > I would like to get your ACK, so I'm trying to improve it.
> >> > ---
> >> >  fs/smb/client/cifsproto.h | 2 +-
> >> >  fs/smb/client/connect.c   | 2 +-
> >> >  fs/smb/client/dfs_cache.c | 2 +-
> >> >  fs/smb/client/inode.c     | 6 +++---
> >> >  fs/smb/client/misc.c      | 6 +++++-
> >> >  fs/smb/client/readdir.c   | 4 ++--
> >> >  6 files changed, 13 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
> >> > index d550662b4e72..07a67c8c37ce 100644
> >> > --- a/fs/smb/client/cifsproto.h
> >> > +++ b/fs/smb/client/cifsproto.h
> >> > @@ -586,9 +586,9 @@ extern int cifs_do_set_acl(const unsigned int xid, struct cifs_tcon *tcon,
> >> >  			   const struct nls_table *nls_codepage, int remap);
> >> >  extern int CIFSGetExtAttr(const unsigned int xid, struct cifs_tcon *tcon,
> >> >  			const int netfid, __u64 *pExtAttrBits, __u64 *pMask);
> >> >  #endif /* CIFS_ALLOW_INSECURE_LEGACY */
> >> > -extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb);
> >> > +extern void cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc);
> >> >  extern bool couldbe_mf_symlink(const struct cifs_fattr *fattr);
> >> >  extern int check_mf_symlink(unsigned int xid, struct cifs_tcon *tcon,
> >> >  			      struct cifs_sb_info *cifs_sb,
> >> >  			      struct cifs_fattr *fattr,
> >> > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> >> > index 6bf04d9a5491..819721dfd5bb 100644
> >> > --- a/fs/smb/client/connect.c
> >> > +++ b/fs/smb/client/connect.c
> >> > @@ -3907,9 +3907,9 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
> >> >  	/*
> >> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
> >> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
> >> >  	 */
> >> > -	cifs_autodisable_serverino(cifs_sb);
> >> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
> >> 
> >> We are mounting an DFS share, not reconnecting.  The message is
> >> misleading.
> >
> > I mostly copied the comment above the cifs_autodisable_serverino() call.
> > Does it mean that the existing comment about reconnecting is wrong too?
> 
> The comment is trying to say why it disabled 'serverino'.  DFS failover
> may potentially connect to a different server and share, hence the inode
> numbers will no longer be valid.  The function is also called
> cifs_mount().

Ok. What do you suggest to put into the message? It would be good to not
have it too long, but also to be useful.

> >> >  	/*
> >> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
> >> >  	 * that have different prefix paths.
> >> >  	 */
> >> > diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
> >> > index 4dada26d56b5..c3fe85c31e2b 100644
> >> > --- a/fs/smb/client/dfs_cache.c
> >> > +++ b/fs/smb/client/dfs_cache.c
> >> > @@ -1288,9 +1288,9 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
> >> >  	/*
> >> >  	 * After reconnecting to a different server, unique ids won't match anymore, so we disable
> >> >  	 * serverino. This prevents dentry revalidation to think the dentry are stale (ESTALE).
> >> >  	 */
> >> > -	cifs_autodisable_serverino(cifs_sb);
> >> > +	cifs_autodisable_serverino(cifs_sb, "Reconnecting to different server, inode numbers won't match anymore", 0);
> >> 
> >> Ditto.
> >> 
> >> >  	/*
> >> >  	 * Force the use of prefix path to support failover on DFS paths that resolve to targets
> >> >  	 * that have different prefix paths.
> >> >  	 */
> >> > diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> >> > index cd06598eacbd..b1c6e3986278 100644
> >> > --- a/fs/smb/client/inode.c
> >> > +++ b/fs/smb/client/inode.c
> >> > @@ -1076,9 +1076,9 @@ static void cifs_set_fattr_ino(int xid, struct cifs_tcon *tcon, struct super_blo
> >> >  		if (*inode)
> >> >  			fattr->cf_uniqueid = CIFS_I(*inode)->uniqueid;
> >> >  		else {
> >> >  			fattr->cf_uniqueid = iunique(sb, ROOT_I);
> >> > -			cifs_autodisable_serverino(cifs_sb);
> >> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via get_srv_inum", rc);
> >> 
> >> Looks good.
> >> 
> >> >  		}
> >> >  		return;
> >> >  	}
> >> >  
> >> > @@ -1529,9 +1529,9 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr)
> >> >  		if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) {
> >> >  			fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION;
> >> >  
> >> >  			if (inode_has_hashed_dentries(inode)) {
> >> > -				cifs_autodisable_serverino(CIFS_SB(sb));
> >> > +				cifs_autodisable_serverino(CIFS_SB(sb), "Inode number collision detected", 0);
> >> 
> >> Looks good.
> >> 
> >> >  				iput(inode);
> >> >  				fattr->cf_uniqueid = iunique(sb, ROOT_I);
> >> >  				goto retry_iget5_locked;
> >> >  			}
> >> > @@ -1596,9 +1596,9 @@ struct inode *cifs_root_iget(struct super_block *sb)
> >> >  iget_root:
> >> >  	if (!rc) {
> >> >  		if (fattr.cf_flags & CIFS_FATTR_JUNCTION) {
> >> >  			fattr.cf_flags &= ~CIFS_FATTR_JUNCTION;
> >> > -			cifs_autodisable_serverino(cifs_sb);
> >> > +			cifs_autodisable_serverino(cifs_sb, "Cannot retrieve attributes for junction point", rc);
> >> 
> >> This has nothing to do with not being able to retrieve attributes.  It
> >> is simply disabling 'serverino' to prevent inode collisions with
> >> surrogate reparse points (automounts).  This should also printed with
> >> FYI.
> >
> > Ok. So then I misunderstood the code around. Do you know when exactly
> > can this case happen? And it is really a problem? Because name surrogate
> > reparse point already creates a new mount hierarchy for which is
> > generated new st_dev major and minor numbers and hence inode collisions
> > should not happen (they do not share st_dev anymore).
> 
> There was a bug report of someone having inode collisions in a share
> that had a reparse mount point, so the server was returning duplicate
> inode numbers for files inside that share.  That is why we set those
> directories as automounts and then disable 'serverino' only for them, so
> the parent mount can still rely on the inode numbers from the server and
> having hardlinks working.
> 
> Note that disabling 'serverino' means that the client won't trust the
> inode numbers from the server and it will generate its own inode
> numbers.  I don't understand why st_dev is relevant here.

Different st_dev values means that they are different filesystem / mounts
So e.g. /dev/ has different st_dev value than the /mnt/smb. And hence
the same inode number can be in /dev/ and /mnt/smb, and they will not
conflict.

And same applies for name surrogate reparse points. Linux for more major
versions is reporting different st_dev value for name surrogate reparse
points, so their inode number would not conflict with the parent / main
mount point.

> >
> >> >  		}
> >> >  		inode = cifs_iget(sb, &fattr);
> >> >  	}
> >> >  
> >> > diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
> >> > index e77017f47084..409277883e8a 100644
> >> > --- a/fs/smb/client/misc.c
> >> > +++ b/fs/smb/client/misc.c
> >> > @@ -551,9 +551,9 @@ dump_smb(void *buf, int smb_buf_length)
> >> >  		       smb_buf_length, true);
> >> >  }
> >> >  
> >> >  void
> >> > -cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> >> > +cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb, const char *reason, int rc)
> >> >  {
> >> >  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
> >> >  		struct cifs_tcon *tcon = NULL;
> >> >  
> >> > @@ -561,8 +561,12 @@ cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> >> >  			tcon = cifs_sb_master_tcon(cifs_sb);
> >> >  
> >> >  		cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
> >> >  		cifs_sb->mnt_cifs_serverino_autodisabled = true;
> >> > +		if (rc)
> >> > +			cifs_dbg(VFS, "%s: %d\n", reason, rc);
> >> > +		else
> >> > +			cifs_dbg(VFS, "%s\n", reason);
> >> >  		cifs_dbg(VFS, "Autodisabling the use of server inode numbers on %s\n",
> >> >  			 tcon ? tcon->tree_name : "new server");
> >> >  		cifs_dbg(VFS, "The server doesn't seem to support them properly or the files might be on different servers (DFS)\n");
> >> >  		cifs_dbg(VFS, "Hardlinks will not be recognized on this mount. Consider mounting with the \"noserverino\" option to silence this message.\n");
> >> > diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c
> >> > index 787d6bcb5d1d..06e90921f751 100644
> >> > --- a/fs/smb/client/readdir.c
> >> > +++ b/fs/smb/client/readdir.c
> >> > @@ -412,9 +412,9 @@ _initiate_cifs_search(const unsigned int xid, struct file *file,
> >> >  	if (rc == 0) {
> >> >  		cifsFile->invalidHandle = false;
> >> >  	} else if ((rc == -EOPNOTSUPP) &&
> >> >  		   (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> >> > -		cifs_autodisable_serverino(cifs_sb);
> >> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number via query_dir_first", rc);
> >> 
> >> Looks good.
> >> 
> >> >  		goto ffirst_retry;
> >> >  	}
> >> >  error_exit:
> >> >  	cifs_put_tlink(tlink);
> >> > @@ -1006,9 +1006,9 @@ static int cifs_filldir(char *find_entry, struct file *file,
> >> >  	if (de.ino && (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) {
> >> >  		fattr.cf_uniqueid = de.ino;
> >> >  	} else {
> >> >  		fattr.cf_uniqueid = iunique(sb, ROOT_I);
> >> > -		cifs_autodisable_serverino(cifs_sb);
> >> > +		cifs_autodisable_serverino(cifs_sb, "Cannot retrieve inode number", 0);
> >> 
> >> Perhaps also mention which function it wasn't able to retrieve inode
> >> number from like above?
> >
> > I quickly look at this code around and I was not able to figure out what
> > is that function which was not able to retrieve inode. So I did not
> > write it into the message. Do you know, or could you figure out what is
> > that function / callback?
> 
> "Cannot retrieve inode number from readdir"

Ok. But what is the callback which caused this issue for readdir here?
In the previous one, it was server->ops->query_dir_first.