[PATCH 0/3] bcachefs: Fix some hard link count issues

public inbox for linux-bcachefs@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] bcachefs: Fix some hard link count issues
@ 2025-09-26  2:21 Youling Tang
  2025-09-26  2:21 ` [PATCH 1/3] bcachefs: return -EMLINK instead of -EINVAL when hard link count exceeds limit Youling Tang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Youling Tang @ 2025-09-26  2:21 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: linux-bcachefs, linux-kernel, youling.tang, Youling Tang

From: Youling Tang <tangyouling@kylinos.cn>

- Improves error reporting by returning the semantically correct
  -EMLINK error code.
- Fixes the maximum link count validation logic during hard link
  creation.
- Consolidates link count checks at the VFS layer for better
  maintainability.

NOTE: Patch3 alone can resolve this series of issues, but patches1-2
are retained to better trace the problem origins.

Youling Tang (3):
  bcachefs: return -EMLINK instead of -EINVAL when hard link count
    exceeds limit
  bcachefs: Fix maximum link count check when creating hard links
  bcachefs: Move the link counting check to the VFS layer

 fs/bcachefs/bcachefs.h |  1 +
 fs/bcachefs/fs.c       |  1 +
 fs/bcachefs/inode.c    | 10 ++--------
 fs/bcachefs/inode.h    |  2 +-
 fs/bcachefs/namei.c    |  4 +---
 5 files changed, 6 insertions(+), 12 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] bcachefs: return -EMLINK instead of -EINVAL when hard link count exceeds limit
  2025-09-26  2:21 [PATCH 0/3] bcachefs: Fix some hard link count issues Youling Tang
@ 2025-09-26  2:21 ` Youling Tang
  2025-09-26  2:21 ` [PATCH 2/3] bcachefs: Fix maximum link count check when creating hard links Youling Tang
  2025-09-26  2:21 ` [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer Youling Tang
  2 siblings, 0 replies; 6+ messages in thread
From: Youling Tang @ 2025-09-26  2:21 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: linux-bcachefs, linux-kernel, youling.tang, Youling Tang

From: Youling Tang <tangyouling@kylinos.cn>

Currently bcachefs returns -EINVAL when the hard link count reaches
U32_MAX. However, -EINVAL is a generic invalid argument error that
doesn't accurately convey the specific "too many links" condition.

This patch changes the error return code from -EINVAL to -EMLINK
when the hard link count limit is exceeded, providing more precise
error information to userspace and making it consistent with other
filesystems' behavior.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/bcachefs/errcode.h | 1 +
 fs/bcachefs/inode.c   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h
index acc3b7b67704..b22a694ec750 100644
--- a/fs/bcachefs/errcode.h
+++ b/fs/bcachefs/errcode.h
@@ -215,6 +215,7 @@
 	x(EINVAL,			varint_decode_error)			\
 	x(EINVAL,			erasure_coding_found_btree_node)	\
 	x(EINVAL,			option_negative)			\
+	x(EMLINK,			too_many_links)				\
 	x(EOPNOTSUPP,			may_not_use_incompat_feature)		\
 	x(EROFS,			erofs_trans_commit)			\
 	x(EROFS,			erofs_no_writes)			\
diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c
index ef4cc7395b86..5765144b4d65 100644
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -1193,7 +1193,7 @@ int bch2_inode_nlink_inc(struct bch_inode_unpacked *bi)
 		bi->bi_flags &= ~BCH_INODE_unlinked;
 	else {
 		if (bi->bi_nlink == U32_MAX)
-			return -EINVAL;
+			return -BCH_ERR_too_many_links;
 
 		bi->bi_nlink++;
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] bcachefs: Fix maximum link count check when creating hard links
  2025-09-26  2:21 [PATCH 0/3] bcachefs: Fix some hard link count issues Youling Tang
  2025-09-26  2:21 ` [PATCH 1/3] bcachefs: return -EMLINK instead of -EINVAL when hard link count exceeds limit Youling Tang
@ 2025-09-26  2:21 ` Youling Tang
  2025-09-26  2:21 ` [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer Youling Tang
  2 siblings, 0 replies; 6+ messages in thread
From: Youling Tang @ 2025-09-26  2:21 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: linux-bcachefs, linux-kernel, youling.tang, Youling Tang

From: Youling Tang <tangyouling@kylinos.cn>

When I changed the maximum link count U32_MAX to 4 for testing purposes, I
discovered that for regular files, the maximum number of hard links created
could actually reach 5 (`inode->i_nlink`).

This occurs because `bi->bi_nlink` does not represent the actual `inode->i_nlink`
value, but rather equals `inode->i_nlink - nlink_bias(bi->bi_mode)`. Therefore,
the `bi->bi_nlink` check in bch2_inode_nlink_inc() needs to be corrected.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
NOTE: If pathconf is to be added to support _PC_LINK_MAX for bcachefs in
libc later, BCH_LINK_MAX can be defined as ((1U << 31) - 1U) like xfs.

 fs/bcachefs/bcachefs.h | 1 +
 fs/bcachefs/inode.c    | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/bcachefs/bcachefs.h b/fs/bcachefs/bcachefs.h
index ddfacad0f70c..9d5e6866b1b6 100644
--- a/fs/bcachefs/bcachefs.h
+++ b/fs/bcachefs/bcachefs.h
@@ -714,6 +714,7 @@ struct btree_debug {
 	unsigned		id;
 };
 
+#define BCH_LINK_MAX	U32_MAX
 #define BCH_TRANSACTIONS_NR 128
 
 struct btree_transaction_stats {
diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c
index 5765144b4d65..eedffb505517 100644
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -1192,7 +1192,7 @@ int bch2_inode_nlink_inc(struct bch_inode_unpacked *bi)
 	if (bi->bi_flags & BCH_INODE_unlinked)
 		bi->bi_flags &= ~BCH_INODE_unlinked;
 	else {
-		if (bi->bi_nlink == U32_MAX)
+		if (bi->bi_nlink == BCH_LINK_MAX - nlink_bias(bi->bi_mode))
 			return -BCH_ERR_too_many_links;
 
 		bi->bi_nlink++;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer
  2025-09-26  2:21 [PATCH 0/3] bcachefs: Fix some hard link count issues Youling Tang
  2025-09-26  2:21 ` [PATCH 1/3] bcachefs: return -EMLINK instead of -EINVAL when hard link count exceeds limit Youling Tang
  2025-09-26  2:21 ` [PATCH 2/3] bcachefs: Fix maximum link count check when creating hard links Youling Tang
@ 2025-09-26  2:21 ` Youling Tang
  2025-09-26  3:42   ` Kent Overstreet
  2 siblings, 1 reply; 6+ messages in thread
From: Youling Tang @ 2025-09-26  2:21 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: linux-bcachefs, linux-kernel, youling.tang, Youling Tang

From: Youling Tang <tangyouling@kylinos.cn>

Currently bcachefs only performs link count checks during link operations,
during rename and mkdir operations, the link count should also be checked.

This patch moves the checks to the vfs_{link,rename,mkdir} functions when
sb->s_max_links is set, eliminating the need for filesystem-specific checks.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/bcachefs/errcode.h |  1 -
 fs/bcachefs/fs.c      |  1 +
 fs/bcachefs/inode.c   | 10 ++--------
 fs/bcachefs/inode.h   |  2 +-
 fs/bcachefs/namei.c   |  4 +---
 5 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h
index b22a694ec750..acc3b7b67704 100644
--- a/fs/bcachefs/errcode.h
+++ b/fs/bcachefs/errcode.h
@@ -215,7 +215,6 @@
 	x(EINVAL,			varint_decode_error)			\
 	x(EINVAL,			erasure_coding_found_btree_node)	\
 	x(EINVAL,			option_negative)			\
-	x(EMLINK,			too_many_links)				\
 	x(EOPNOTSUPP,			may_not_use_incompat_feature)		\
 	x(EROFS,			erofs_trans_commit)			\
 	x(EROFS,			erofs_no_writes)			\
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 687af0eea0c2..6b60c97c5610 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -2526,6 +2526,7 @@ static int bch2_fs_get_tree(struct fs_context *fc)
 	sb->s_time_gran		= c->sb.nsec_per_time_unit;
 	sb->s_time_min		= div_s64(S64_MIN, c->sb.time_units_per_sec) + 1;
 	sb->s_time_max		= div_s64(S64_MAX, c->sb.time_units_per_sec);
+	sb->s_max_links		= BCH_LINK_MAX;
 	super_set_uuid(sb, c->sb.user_uuid.b, sizeof(c->sb.user_uuid));
 
 	if (c->sb.multi_device)
diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c
index eedffb505517..20e58258c813 100644
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -1187,18 +1187,12 @@ int bch2_inode_rm(struct bch_fs *c, subvol_inum inum)
 	return ret;
 }
 
-int bch2_inode_nlink_inc(struct bch_inode_unpacked *bi)
+void bch2_inode_nlink_inc(struct bch_inode_unpacked *bi)
 {
 	if (bi->bi_flags & BCH_INODE_unlinked)
 		bi->bi_flags &= ~BCH_INODE_unlinked;
-	else {
-		if (bi->bi_nlink == BCH_LINK_MAX - nlink_bias(bi->bi_mode))
-			return -BCH_ERR_too_many_links;
-
+	else
 		bi->bi_nlink++;
-	}
-
-	return 0;
 }
 
 void bch2_inode_nlink_dec(struct btree_trans *trans, struct bch_inode_unpacked *bi)
diff --git a/fs/bcachefs/inode.h b/fs/bcachefs/inode.h
index b8ec3e628d90..99de17e9f32c 100644
--- a/fs/bcachefs/inode.h
+++ b/fs/bcachefs/inode.h
@@ -285,7 +285,7 @@ static inline void bch2_inode_nlink_set(struct bch_inode_unpacked *bi,
 	}
 }
 
-int bch2_inode_nlink_inc(struct bch_inode_unpacked *);
+void bch2_inode_nlink_inc(struct bch_inode_unpacked *);
 void bch2_inode_nlink_dec(struct btree_trans *, struct bch_inode_unpacked *);
 
 struct bch_opts bch2_inode_opts_to_opts(struct bch_inode_unpacked *);
diff --git a/fs/bcachefs/namei.c b/fs/bcachefs/namei.c
index c3f87c59922d..42e06baa2e43 100644
--- a/fs/bcachefs/namei.c
+++ b/fs/bcachefs/namei.c
@@ -221,9 +221,7 @@ int bch2_link_trans(struct btree_trans *trans,
 		return ret;
 
 	inode_u->bi_ctime = now;
-	ret = bch2_inode_nlink_inc(inode_u);
-	if (ret)
-		goto err;
+	bch2_inode_nlink_inc(inode_u);
 
 	ret = bch2_inode_peek(trans, &dir_iter, dir_u, dir, BTREE_ITER_intent);
 	if (ret)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer
  2025-09-26  2:21 ` [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer Youling Tang
@ 2025-09-26  3:42   ` Kent Overstreet
  2025-09-26  4:22     ` Youling Tang
  0 siblings, 1 reply; 6+ messages in thread
From: Kent Overstreet @ 2025-09-26  3:42 UTC (permalink / raw)
  To: Youling Tang; +Cc: linux-bcachefs, linux-kernel, Youling Tang

On Fri, Sep 26, 2025 at 10:21:50AM +0800, Youling Tang wrote:
> From: Youling Tang <tangyouling@kylinos.cn>
> 
> Currently bcachefs only performs link count checks during link operations,
> during rename and mkdir operations, the link count should also be checked.
> 
> This patch moves the checks to the vfs_{link,rename,mkdir} functions when
> sb->s_max_links is set, eliminating the need for filesystem-specific checks.
> 
> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>

I applied the other two patches, but not this one. We can't rely on the
VFS for checks like this, there's lots of routes to modifying the
filesystem that don't go through the VFS.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer
  2025-09-26  3:42   ` Kent Overstreet
@ 2025-09-26  4:22     ` Youling Tang
  0 siblings, 0 replies; 6+ messages in thread
From: Youling Tang @ 2025-09-26  4:22 UTC (permalink / raw)
  To: Kent Overstreet; +Cc: linux-bcachefs, linux-kernel, Youling Tang

Hi, Kent
On 9/26/25 11:42, Kent Overstreet wrote:
> On Fri, Sep 26, 2025 at 10:21:50AM +0800, Youling Tang wrote:
>> From: Youling Tang <tangyouling@kylinos.cn>
>>
>> Currently bcachefs only performs link count checks during link operations,
>> during rename and mkdir operations, the link count should also be checked.
>>
>> This patch moves the checks to the vfs_{link,rename,mkdir} functions when
>> sb->s_max_links is set, eliminating the need for filesystem-specific checks.
>>
>> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> I applied the other two patches, but not this one. We can't rely on the
> VFS for checks like this, there's lots of routes to modifying the
> filesystem that don't go through the VFS.
```
vfs_link
     bch2_link
         __bch2_link
             bch2_link_trans
                 bch2_inode_nlink_inc

bch2_symlink
     __bch2_link
```
I have traversed the bcachefs code and found that bch2_inode_nlink_inc is
called only once by bch2_link_trans, which in turn is called only once
by __bch2_link.

Although __bch2_link is also called by bch2_symlink, symlink operations
don't involve i_nlink values. Therefore, all effective calls to
bch2_inode_nlink_incoriginate from vfs_link. Please let me know if
there are any calls from other sources.

Even without moving to the VFS layer, do we still need to add link count
validation in bch2_{mkdir, rename}?

Thanks,
Youling.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-09-26  4:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-26  2:21 [PATCH 0/3] bcachefs: Fix some hard link count issues Youling Tang
2025-09-26  2:21 ` [PATCH 1/3] bcachefs: return -EMLINK instead of -EINVAL when hard link count exceeds limit Youling Tang
2025-09-26  2:21 ` [PATCH 2/3] bcachefs: Fix maximum link count check when creating hard links Youling Tang
2025-09-26  2:21 ` [PATCH 3/3] bcachefs: Move the link counting check to the VFS layer Youling Tang
2025-09-26  3:42   ` Kent Overstreet
2025-09-26  4:22     ` Youling Tang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox