Skip to content

Commit

Permalink
nilfs2: fix gc failure on volumes keeping numerous snapshots
Browse files Browse the repository at this point in the history 
This resolves the following failure of nilfs2 cleaner daemon:

 nilfs_cleanerd[20670]: cannot clean segments: No such file or directory
 nilfs_cleanerd[20670]: shutdown

When creating thousands of snapshots, the cleaner daemon had rarely died
as above due to an error returned from the kernel code.

After applying the recent patch which fixed memory allocation problems in
ioctl (Message-Id: <[email protected]>), the
problem gets more frequent.

It turned out to be a bug of nilfs_ioctl_wrap_copy function and one of its
callback routines to read out information of snapshots; if the
nilfs_ioctl_wrap_copy function divided a large read request into multiple
requests, the second and later requests have failed since a restart
position on snapshot meta data was not properly set forward.

It's a deficiency of the callback interface that cannot pass the restart
position among multiple requests.  This patch fixes the issue by allowing
nilfs_ioctl_wrap_copy and snapshot read functions to exchange a position
argument.

Signed-off-by: Ryusuke Konishi <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
@konis @torvalds
konis authored and torvalds committed Apr 7, 2009
1 parent 85ef9ce commit b028fcf
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 26 deletions.
24 changes: 14 additions & 10 deletions fs/nilfs2/cpfile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,20 +422,20 @@ static ssize_t nilfs_cpfile_do_get_cpinfo(struct inode *cpfile, __u64 cno,
return ret;
}

static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 cno,
static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 *cnop,
struct nilfs_cpinfo *ci, size_t nci)
{
struct buffer_head *bh;
struct nilfs_cpfile_header *header;
struct nilfs_checkpoint *cp;
__u64 curr, next;
__u64 curr = *cnop, next;
unsigned long curr_blkoff, next_blkoff;
void *kaddr;
int n, ret;

down_read(&NILFS_MDT(cpfile)->mi_sem);

if (cno == 0) {
if (curr == 0) {
ret = nilfs_cpfile_get_header_block(cpfile, &bh);
if (ret < 0)
goto out;
Expand All @@ -448,8 +448,11 @@ static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 cno,
ret = 0;
goto out;
}
} else
curr = cno;
} else if (unlikely(curr == ~(__u64)0)) {
ret = 0;
goto out;
}

curr_blkoff = nilfs_cpfile_get_blkoff(cpfile, curr);
ret = nilfs_cpfile_get_checkpoint_block(cpfile, curr, 0, &bh);
if (ret < 0)
Expand All @@ -461,7 +464,7 @@ static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 cno,
nilfs_cpfile_checkpoint_to_cpinfo(cpfile, cp, &ci[n]);
next = le64_to_cpu(cp->cp_snapshot_list.ssl_next);
if (next == 0) {
curr = next;
curr = ~(__u64)0; /* Terminator */
n++;
break;
}
Expand All @@ -480,6 +483,7 @@ static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 cno,
}
kunmap_atomic(kaddr, KM_USER0);
brelse(bh);
*cnop = curr;
ret = n;

out:
Expand All @@ -494,15 +498,15 @@ static ssize_t nilfs_cpfile_do_get_ssinfo(struct inode *cpfile, __u64 cno,
* @ci:
* @nci:
*/
ssize_t nilfs_cpfile_get_cpinfo(struct inode *cpfile,
__u64 cno, int mode,

ssize_t nilfs_cpfile_get_cpinfo(struct inode *cpfile, __u64 *cnop, int mode,
struct nilfs_cpinfo *ci, size_t nci)
{
switch (mode) {
case NILFS_CHECKPOINT:
return nilfs_cpfile_do_get_cpinfo(cpfile, cno, ci, nci);
return nilfs_cpfile_do_get_cpinfo(cpfile, *cnop, ci, nci);
case NILFS_SNAPSHOT:
return nilfs_cpfile_do_get_ssinfo(cpfile, cno, ci, nci);
return nilfs_cpfile_do_get_ssinfo(cpfile, cnop, ci, nci);
default:
return -EINVAL;
}
Expand Down
2 changes: 1 addition & 1 deletion fs/nilfs2/cpfile.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ int nilfs_cpfile_delete_checkpoint(struct inode *, __u64);
int nilfs_cpfile_change_cpmode(struct inode *, __u64, int);
int nilfs_cpfile_is_snapshot(struct inode *, __u64);
int nilfs_cpfile_get_stat(struct inode *, struct nilfs_cpstat *);
ssize_t nilfs_cpfile_get_cpinfo(struct inode *, __u64, int,
ssize_t nilfs_cpfile_get_cpinfo(struct inode *, __u64 *, int,
struct nilfs_cpinfo *, size_t);

#endif /* _NILFS_CPFILE_H */
38 changes: 23 additions & 15 deletions fs/nilfs2/ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@
static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
struct nilfs_argv *argv, int dir,
ssize_t (*dofunc)(struct the_nilfs *,
int, int,
__u64 *, int,
void *, size_t, size_t))
{
void *buf;
size_t maxmembs, total, n;
ssize_t nr;
int ret, i;
__u64 pos, ppos;

if (argv->v_nmembs == 0)
return 0;
Expand All @@ -58,6 +59,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,

ret = 0;
total = 0;
pos = argv->v_index;
for (i = 0; i < argv->v_nmembs; i += n) {
n = (argv->v_nmembs - i < maxmembs) ?
argv->v_nmembs - i : maxmembs;
Expand All @@ -68,8 +70,9 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
ret = -EFAULT;
break;
}
nr = (*dofunc)(nilfs, argv->v_index + i, argv->v_flags, buf,
argv->v_size, n);
ppos = pos;
nr = (*dofunc)(nilfs, &pos, argv->v_flags, buf, argv->v_size,
n);
if (nr < 0) {
ret = nr;
break;
Expand All @@ -82,6 +85,10 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
break;
}
total += nr;
if ((size_t)nr < n)
break;
if (pos == ppos)
pos += n;
}
argv->v_nmembs = total;

Expand Down Expand Up @@ -138,10 +145,10 @@ nilfs_ioctl_delete_checkpoint(struct inode *inode, struct file *filp,
}

static ssize_t
nilfs_ioctl_do_get_cpinfo(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_get_cpinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
return nilfs_cpfile_get_cpinfo(nilfs->ns_cpfile, index, flags, buf,
return nilfs_cpfile_get_cpinfo(nilfs->ns_cpfile, posp, flags, buf,
nmembs);
}

Expand Down Expand Up @@ -186,10 +193,10 @@ static int nilfs_ioctl_get_cpstat(struct inode *inode, struct file *filp,
}

static ssize_t
nilfs_ioctl_do_get_suinfo(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_get_suinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
return nilfs_sufile_get_suinfo(nilfs->ns_sufile, index, buf, nmembs);
return nilfs_sufile_get_suinfo(nilfs->ns_sufile, *posp, buf, nmembs);
}

static int nilfs_ioctl_get_suinfo(struct inode *inode, struct file *filp,
Expand Down Expand Up @@ -233,7 +240,7 @@ static int nilfs_ioctl_get_sustat(struct inode *inode, struct file *filp,
}

static ssize_t
nilfs_ioctl_do_get_vinfo(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_get_vinfo(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
return nilfs_dat_get_vinfo(nilfs_dat_inode(nilfs), buf, nmembs);
Expand Down Expand Up @@ -262,7 +269,7 @@ static int nilfs_ioctl_get_vinfo(struct inode *inode, struct file *filp,
}

static ssize_t
nilfs_ioctl_do_get_bdescs(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_get_bdescs(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
struct inode *dat = nilfs_dat_inode(nilfs);
Expand Down Expand Up @@ -341,7 +348,7 @@ static int nilfs_ioctl_move_inode_block(struct inode *inode,
}

static ssize_t
nilfs_ioctl_do_move_blocks(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_move_blocks(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
struct inode *inode;
Expand Down Expand Up @@ -413,7 +420,7 @@ static inline int nilfs_ioctl_move_blocks(struct the_nilfs *nilfs,
}

static ssize_t
nilfs_ioctl_do_delete_checkpoints(struct the_nilfs *nilfs, int index,
nilfs_ioctl_do_delete_checkpoints(struct the_nilfs *nilfs, __u64 *posp,
int flags, void *buf, size_t size,
size_t nmembs)
{
Expand All @@ -439,7 +446,7 @@ static inline int nilfs_ioctl_delete_checkpoints(struct the_nilfs *nilfs,
}

static ssize_t
nilfs_ioctl_do_free_vblocknrs(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_free_vblocknrs(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
int ret = nilfs_dat_freev(nilfs_dat_inode(nilfs), buf, nmembs);
Expand All @@ -456,8 +463,9 @@ static inline int nilfs_ioctl_free_vblocknrs(struct the_nilfs *nilfs,
}

static ssize_t
nilfs_ioctl_do_mark_blocks_dirty(struct the_nilfs *nilfs, int index, int flags,
void *buf, size_t size, size_t nmembs)
nilfs_ioctl_do_mark_blocks_dirty(struct the_nilfs *nilfs, __u64 *posp,
int flags, void *buf, size_t size,
size_t nmembs)
{
struct inode *dat = nilfs_dat_inode(nilfs);
struct nilfs_bmap *bmap = NILFS_I(dat)->i_bmap;
Expand Down Expand Up @@ -506,7 +514,7 @@ static inline int nilfs_ioctl_mark_blocks_dirty(struct the_nilfs *nilfs,
}

static ssize_t
nilfs_ioctl_do_free_segments(struct the_nilfs *nilfs, int index, int flags,
nilfs_ioctl_do_free_segments(struct the_nilfs *nilfs, __u64 *posp, int flags,
void *buf, size_t size, size_t nmembs)
{
struct nilfs_sb_info *sbi = nilfs_get_writer(nilfs);
Expand Down

0 comments on commit b028fcf

Please sign in to comment.