Skip to content

Commit e1b4c6a

Browse files
author
Al Viro
committed
shmem: fix recovery on rename failures
maple_tree insertions can fail if we are seriously short on memory; simple_offset_rename() does not recover well if it runs into that. The same goes for simple_offset_rename_exchange(). Moreover, shmem_whiteout() expects that if it succeeds, the caller will progress to d_move(), i.e. that shmem_rename2() won't fail past the successful call of shmem_whiteout(). Not hard to fix, fortunately - mtree_store() can't fail if the index we are trying to store into is already present in the tree as a singleton. For simple_offset_rename_exchange() that's enough - we just need to be careful about the order of operations. For simple_offset_rename() solution is to preinsert the target into the tree for new_dir; the rest can be done without any potentially failing operations. That preinsertion has to be done in shmem_rename2() rather than in simple_offset_rename() itself - otherwise we'd need to deal with the possibility of failure after successful shmem_whiteout(). Fixes: a2e4595 ("shmem: stable directory offsets") Reviewed-by: Christian Brauner <brauner@kernel.org> Reviewed-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
1 parent 3010f06 commit e1b4c6a

3 files changed

Lines changed: 35 additions & 35 deletions

File tree

fs/libfs.c

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -346,22 +346,22 @@ void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry)
346346
* User space expects the directory offset value of the replaced
347347
* (new) directory entry to be unchanged after a rename.
348348
*
349-
* Returns zero on success, a negative errno value on failure.
349+
* Caller must have grabbed a slot for new_dentry in the maple_tree
350+
* associated with new_dir, even if dentry is negative.
350351
*/
351-
int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
352-
struct inode *new_dir, struct dentry *new_dentry)
352+
void simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
353+
struct inode *new_dir, struct dentry *new_dentry)
353354
{
354355
struct offset_ctx *old_ctx = old_dir->i_op->get_offset_ctx(old_dir);
355356
struct offset_ctx *new_ctx = new_dir->i_op->get_offset_ctx(new_dir);
356357
long new_offset = dentry2offset(new_dentry);
357358

358-
simple_offset_remove(old_ctx, old_dentry);
359+
if (WARN_ON(!new_offset))
360+
return;
359361

360-
if (new_offset) {
361-
offset_set(new_dentry, 0);
362-
return simple_offset_replace(new_ctx, old_dentry, new_offset);
363-
}
364-
return simple_offset_add(new_ctx, old_dentry);
362+
simple_offset_remove(old_ctx, old_dentry);
363+
offset_set(new_dentry, 0);
364+
WARN_ON(simple_offset_replace(new_ctx, old_dentry, new_offset));
365365
}
366366

367367
/**
@@ -388,31 +388,23 @@ int simple_offset_rename_exchange(struct inode *old_dir,
388388
long new_index = dentry2offset(new_dentry);
389389
int ret;
390390

391-
simple_offset_remove(old_ctx, old_dentry);
392-
simple_offset_remove(new_ctx, new_dentry);
391+
if (WARN_ON(!old_index || !new_index))
392+
return -EINVAL;
393393

394-
ret = simple_offset_replace(new_ctx, old_dentry, new_index);
395-
if (ret)
396-
goto out_restore;
394+
ret = mtree_store(&new_ctx->mt, new_index, old_dentry, GFP_KERNEL);
395+
if (WARN_ON(ret))
396+
return ret;
397397

398-
ret = simple_offset_replace(old_ctx, new_dentry, old_index);
399-
if (ret) {
400-
simple_offset_remove(new_ctx, old_dentry);
401-
goto out_restore;
398+
ret = mtree_store(&old_ctx->mt, old_index, new_dentry, GFP_KERNEL);
399+
if (WARN_ON(ret)) {
400+
mtree_store(&new_ctx->mt, new_index, new_dentry, GFP_KERNEL);
401+
return ret;
402402
}
403403

404-
ret = simple_rename_exchange(old_dir, old_dentry, new_dir, new_dentry);
405-
if (ret) {
406-
simple_offset_remove(new_ctx, old_dentry);
407-
simple_offset_remove(old_ctx, new_dentry);
408-
goto out_restore;
409-
}
404+
offset_set(old_dentry, new_index);
405+
offset_set(new_dentry, old_index);
406+
simple_rename_exchange(old_dir, old_dentry, new_dir, new_dentry);
410407
return 0;
411-
412-
out_restore:
413-
(void)simple_offset_replace(old_ctx, old_dentry, old_index);
414-
(void)simple_offset_replace(new_ctx, new_dentry, new_index);
415-
return ret;
416408
}
417409

418410
/**

include/linux/fs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3247,7 +3247,7 @@ struct offset_ctx {
32473247
void simple_offset_init(struct offset_ctx *octx);
32483248
int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry);
32493249
void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry);
3250-
int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
3250+
void simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
32513251
struct inode *new_dir, struct dentry *new_dentry);
32523252
int simple_offset_rename_exchange(struct inode *old_dir,
32533253
struct dentry *old_dentry,

mm/shmem.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4038,6 +4038,7 @@ static int shmem_rename2(struct mnt_idmap *idmap,
40384038
{
40394039
struct inode *inode = d_inode(old_dentry);
40404040
int they_are_dirs = S_ISDIR(inode->i_mode);
4041+
bool had_offset = false;
40414042
int error;
40424043

40434044
if (flags & ~(RENAME_NOREPLACE | RENAME_EXCHANGE | RENAME_WHITEOUT))
@@ -4050,16 +4051,23 @@ static int shmem_rename2(struct mnt_idmap *idmap,
40504051
if (!simple_empty(new_dentry))
40514052
return -ENOTEMPTY;
40524053

4054+
error = simple_offset_add(shmem_get_offset_ctx(new_dir), new_dentry);
4055+
if (error == -EBUSY)
4056+
had_offset = true;
4057+
else if (unlikely(error))
4058+
return error;
4059+
40534060
if (flags & RENAME_WHITEOUT) {
40544061
error = shmem_whiteout(idmap, old_dir, old_dentry);
4055-
if (error)
4062+
if (error) {
4063+
if (!had_offset)
4064+
simple_offset_remove(shmem_get_offset_ctx(new_dir),
4065+
new_dentry);
40564066
return error;
4067+
}
40574068
}
40584069

4059-
error = simple_offset_rename(old_dir, old_dentry, new_dir, new_dentry);
4060-
if (error)
4061-
return error;
4062-
4070+
simple_offset_rename(old_dir, old_dentry, new_dir, new_dentry);
40634071
if (d_really_is_positive(new_dentry)) {
40644072
(void) shmem_unlink(new_dir, new_dentry);
40654073
if (they_are_dirs) {

0 commit comments

Comments
 (0)