* [RFC] a couple of i_nlink fixes in btrfs
@ 2011-03-04 17:13 Al Viro
2011-03-04 17:14 ` [PATCH 1/2] btrfs: don't mess with i_nlink of unlocked inode in rename() Al Viro
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Al Viro @ 2011-03-04 17:13 UTC (permalink / raw)
To: chris.mason; +Cc: linux-kernel, linux-btrfs
a) rename() plays with i_nlink of old_inode; bad, since it's not
locked. I'd add a variant of btrfs_unlink_inode() that would leave
btrfs_drop_nlink()/btrfs_update_inode() to callers and use it instead.
b) btrfs_link() doesn't check for i_nlink overflows. I don't
know if there's anything preventing that many links to a file on btrfs,
but if there is, it's at least worth a comment in there...
Please, review; patches in followups or in #btrfs in vfs-2.6.git
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] btrfs: don't mess with i_nlink of unlocked inode in rename() 2011-03-04 17:13 [RFC] a couple of i_nlink fixes in btrfs Al Viro @ 2011-03-04 17:14 ` Al Viro 2011-03-04 17:15 ` [PATCH 2/2] btrfs: check link counter overflow in link(2) Al Viro 2011-03-07 16:58 ` [RFC] a couple of i_nlink fixes in btrfs Chris Mason 2 siblings, 0 replies; 6+ messages in thread From: Al Viro @ 2011-03-04 17:14 UTC (permalink / raw) To: chris.mason; +Cc: linux-kernel, linux-btrfs old_inode is not locked; it's not safe to play with its link count. Instead of bumping it and calling btrfs_unlink_inode(), add a variant of the latter that does not do btrfs_drop_nlink()/ btrfs_update_inode(), call it instead of btrfs_inc_nlink()/ btrfs_unlink_inode() and do btrfs_update_inode() ourselves. Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/btrfs/inode.c | 36 +++++++++++++++++++++++++----------- 1 files changed, 25 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 0efdb65..099d64c 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2634,10 +2634,10 @@ failed: * recovery code. It remove a link in a directory with a given name, and * also drops the back refs in the inode to the directory */ -int btrfs_unlink_inode(struct btrfs_trans_handle *trans, - struct btrfs_root *root, - struct inode *dir, struct inode *inode, - const char *name, int name_len) +static int __btrfs_unlink_inode(struct btrfs_trans_handle *trans, + struct btrfs_root *root, + struct inode *dir, struct inode *inode, + const char *name, int name_len) { struct btrfs_path *path; int ret = 0; @@ -2709,12 +2709,25 @@ err: btrfs_i_size_write(dir, dir->i_size - name_len * 2); inode->i_ctime = dir->i_mtime = dir->i_ctime = CURRENT_TIME; btrfs_update_inode(trans, root, dir); - btrfs_drop_nlink(inode); - ret = btrfs_update_inode(trans, root, inode); out: return ret; } +int btrfs_unlink_inode(struct btrfs_trans_handle *trans, + struct btrfs_root *root, + struct inode *dir, struct inode *inode, + const char *name, int name_len) +{ + int ret; + ret = __btrfs_unlink_inode(trans, root, dir, inode, name, name_len); + if (!ret) { + btrfs_drop_nlink(inode); + ret = btrfs_update_inode(trans, root, inode); + } + return ret; +} + + /* helper to check if there is any shared block in the path */ static int check_path_shared(struct btrfs_root *root, struct btrfs_path *path) @@ -6908,11 +6921,12 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, old_dentry->d_name.name, old_dentry->d_name.len); } else { - btrfs_inc_nlink(old_dentry->d_inode); - ret = btrfs_unlink_inode(trans, root, old_dir, - old_dentry->d_inode, - old_dentry->d_name.name, - old_dentry->d_name.len); + ret = __btrfs_unlink_inode(trans, root, old_dir, + old_dentry->d_inode, + old_dentry->d_name.name, + old_dentry->d_name.len); + if (!ret) + ret = btrfs_update_inode(trans, root, old_inode); } BUG_ON(ret); -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] btrfs: check link counter overflow in link(2) 2011-03-04 17:13 [RFC] a couple of i_nlink fixes in btrfs Al Viro 2011-03-04 17:14 ` [PATCH 1/2] btrfs: don't mess with i_nlink of unlocked inode in rename() Al Viro @ 2011-03-04 17:15 ` Al Viro 2011-03-07 16:58 ` [RFC] a couple of i_nlink fixes in btrfs Chris Mason 2 siblings, 0 replies; 6+ messages in thread From: Al Viro @ 2011-03-04 17:15 UTC (permalink / raw) To: chris.mason; +Cc: linux-kernel, linux-btrfs Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- fs/btrfs/inode.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 099d64c..8a6df03 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4826,6 +4826,9 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir, if (root->objectid != BTRFS_I(inode)->root->objectid) return -EPERM; + if (inode->i_nlink == ~0U) + return -EMLINK; + btrfs_inc_nlink(inode); inode->i_ctime = CURRENT_TIME; -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] a couple of i_nlink fixes in btrfs 2011-03-04 17:13 [RFC] a couple of i_nlink fixes in btrfs Al Viro 2011-03-04 17:14 ` [PATCH 1/2] btrfs: don't mess with i_nlink of unlocked inode in rename() Al Viro 2011-03-04 17:15 ` [PATCH 2/2] btrfs: check link counter overflow in link(2) Al Viro @ 2011-03-07 16:58 ` Chris Mason 2011-03-21 5:17 ` Al Viro 2 siblings, 1 reply; 6+ messages in thread From: Chris Mason @ 2011-03-07 16:58 UTC (permalink / raw) To: Al Viro; +Cc: linux-kernel, linux-btrfs Excerpts from Al Viro's message of 2011-03-04 12:13:53 -0500: > a) rename() plays with i_nlink of old_inode; bad, since it's not > locked. I'd add a variant of btrfs_unlink_inode() that would leave > btrfs_drop_nlink()/btrfs_update_inode() to callers and use it instead. > b) btrfs_link() doesn't check for i_nlink overflows. I don't > know if there's anything preventing that many links to a file on btrfs, > but if there is, it's at least worth a comment in there... > > Please, review; patches in followups or in #btrfs in vfs-2.6.git Thanks, these both look good but I'll test here as well. Are you planning on pushing for .38? -chris ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] a couple of i_nlink fixes in btrfs 2011-03-07 16:58 ` [RFC] a couple of i_nlink fixes in btrfs Chris Mason @ 2011-03-21 5:17 ` Al Viro 2011-03-21 11:37 ` Chris Mason 0 siblings, 1 reply; 6+ messages in thread From: Al Viro @ 2011-03-21 5:17 UTC (permalink / raw) To: Chris Mason; +Cc: linux-kernel, linux-btrfs On Mon, Mar 07, 2011 at 11:58:13AM -0500, Chris Mason wrote: > Thanks, these both look good but I'll test here as well. Are you > planning on pushing for .38? No, but .39 would be nice ;-) Do you want that to go through btrfs tree or through vfs one? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] a couple of i_nlink fixes in btrfs 2011-03-21 5:17 ` Al Viro @ 2011-03-21 11:37 ` Chris Mason 0 siblings, 0 replies; 6+ messages in thread From: Chris Mason @ 2011-03-21 11:37 UTC (permalink / raw) To: Al Viro; +Cc: linux-kernel, linux-btrfs Excerpts from Al Viro's message of 2011-03-21 01:17:25 -0400: > On Mon, Mar 07, 2011 at 11:58:13AM -0500, Chris Mason wrote: > > > Thanks, these both look good but I'll test here as well. Are you > > planning on pushing for .38? > > No, but .39 would be nice ;-) Do you want that to go through btrfs tree > or through vfs one? I'll take them in mine, it'll be easier to put in with all these other changes. -chris ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-21 11:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-04 17:13 [RFC] a couple of i_nlink fixes in btrfs Al Viro 2011-03-04 17:14 ` [PATCH 1/2] btrfs: don't mess with i_nlink of unlocked inode in rename() Al Viro 2011-03-04 17:15 ` [PATCH 2/2] btrfs: check link counter overflow in link(2) Al Viro 2011-03-07 16:58 ` [RFC] a couple of i_nlink fixes in btrfs Chris Mason 2011-03-21 5:17 ` Al Viro 2011-03-21 11:37 ` Chris Mason
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 NNTP newsgroup(s).