* [PATCH mmotm 1/6] nilfs2: remove timedwait ioctl command
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 2/6] nilfs2: use fixed sized types for ioctl structures Ryusuke Konishi
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi
This removes NILFS_IOCTL_TIMEDWAIT command from ioctl interface along
with the related flags and wait queue.
The command is terrible because it just sleeps in the ioctl. I prefer
to avoid this by devising means of event polling in userland program.
By reconsidering the userland GC daemon, I found this is possible
without changing behaviour of the daemon and sacrificing efficiency.
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/ioctl.c | 95 +--------------------------------------------
fs/nilfs2/segment.c | 5 +--
fs/nilfs2/the_nilfs.c | 1 -
fs/nilfs2/the_nilfs.h | 6 ---
include/linux/nilfs2_fs.h | 22 ----------
5 files changed, 2 insertions(+), 127 deletions(-)
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 9e4d9e6..85a291c 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -578,62 +578,9 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
static int nilfs_ioctl_clean_segments(struct inode *inode, struct file *filp,
unsigned int cmd, void __user *argp)
{
- int ret;
-
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
-
- ret = nilfs_clean_segments(inode->i_sb, argp);
- clear_nilfs_cond_nongc_write(NILFS_SB(inode->i_sb)->s_nilfs);
- return ret;
-}
-
-static int nilfs_ioctl_test_cond(struct the_nilfs *nilfs, int cond)
-{
- return (cond & NILFS_TIMEDWAIT_SEG_WRITE) &&
- nilfs_cond_nongc_write(nilfs);
-}
-
-static void nilfs_ioctl_clear_cond(struct the_nilfs *nilfs, int cond)
-{
- if (cond & NILFS_TIMEDWAIT_SEG_WRITE)
- clear_nilfs_cond_nongc_write(nilfs);
-}
-
-static int nilfs_ioctl_timedwait(struct inode *inode, struct file *filp,
- unsigned int cmd, void __user *argp)
-{
- struct the_nilfs *nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
- struct nilfs_wait_cond wc;
- long ret;
-
- if (!capable(CAP_SYS_ADMIN))
- return -EPERM;
- if (copy_from_user(&wc, argp, sizeof(wc)))
- return -EFAULT;
-
- unlock_kernel();
- ret = wc.wc_flags ?
- wait_event_interruptible_timeout(
- nilfs->ns_cleanerd_wq,
- nilfs_ioctl_test_cond(nilfs, wc.wc_cond),
- timespec_to_jiffies(&wc.wc_timeout)) :
- wait_event_interruptible(
- nilfs->ns_cleanerd_wq,
- nilfs_ioctl_test_cond(nilfs, wc.wc_cond));
- lock_kernel();
- nilfs_ioctl_clear_cond(nilfs, wc.wc_cond);
-
- if (ret > 0) {
- jiffies_to_timespec(ret, &wc.wc_timeout);
- if (copy_to_user(argp, &wc, sizeof(wc)))
- return -EFAULT;
- return 0;
- }
- if (ret != 0)
- return -EINTR;
-
- return wc.wc_flags ? -ETIME : 0;
+ return nilfs_clean_segments(inode->i_sb, argp);
}
static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
@@ -679,8 +626,6 @@ int nilfs_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
return nilfs_ioctl_get_bdescs(inode, filp, cmd, argp);
case NILFS_IOCTL_CLEAN_SEGMENTS:
return nilfs_ioctl_clean_segments(inode, filp, cmd, argp);
- case NILFS_IOCTL_TIMEDWAIT:
- return nilfs_ioctl_timedwait(inode, filp, cmd, argp);
case NILFS_IOCTL_SYNC:
return nilfs_ioctl_sync(inode, filp, cmd, argp);
default:
@@ -871,41 +816,6 @@ nilfs_compat_ioctl_clean_segments(struct inode *inode, struct file *filp,
inode, filp, cmd, (unsigned long)uargv);
}
-static int
-nilfs_compat_ioctl_timedwait(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- struct nilfs_wait_cond __user *uwcond;
- struct nilfs_wait_cond32 __user *uwcond32;
- struct timespec ts;
- int cond, flags, ret;
-
- uwcond = compat_alloc_user_space(sizeof(struct nilfs_wait_cond));
- uwcond32 = compat_ptr(arg);
- if (get_user(cond, &uwcond32->wc_cond) ||
- put_user(cond, &uwcond->wc_cond) ||
- get_user(flags, &uwcond32->wc_flags) ||
- put_user(flags, &uwcond->wc_flags) ||
- get_user(ts.tv_sec, &uwcond32->wc_timeout.tv_sec) ||
- get_user(ts.tv_nsec, &uwcond32->wc_timeout.tv_nsec) ||
- put_user(ts.tv_sec, &uwcond->wc_timeout.tv_sec) ||
- put_user(ts.tv_nsec, &uwcond->wc_timeout.tv_nsec))
- return -EFAULT;
-
- ret = nilfs_compat_locked_ioctl(inode, filp, cmd,
- (unsigned long)uwcond);
- if (ret < 0)
- return ret;
-
- if (get_user(ts.tv_sec, &uwcond->wc_timeout.tv_sec) ||
- get_user(ts.tv_nsec, &uwcond->wc_timeout.tv_nsec) ||
- put_user(ts.tv_sec, &uwcond32->wc_timeout.tv_sec) ||
- put_user(ts.tv_nsec, &uwcond32->wc_timeout.tv_nsec))
- return -EFAULT;
-
- return 0;
-}
-
static int nilfs_compat_ioctl_sync(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
{
@@ -943,9 +853,6 @@ long nilfs_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case NILFS_IOCTL32_CLEAN_SEGMENTS:
return nilfs_compat_ioctl_clean_segments(
inode, filp, NILFS_IOCTL_CLEAN_SEGMENTS, arg);
- case NILFS_IOCTL32_TIMEDWAIT:
- return nilfs_compat_ioctl_timedwait(
- inode, filp, NILFS_IOCTL_TIMEDWAIT, arg);
case NILFS_IOCTL_SYNC:
return nilfs_compat_ioctl_sync(inode, filp, cmd, arg);
default:
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 6d66c5c..5db12d7 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2114,11 +2114,8 @@ static void nilfs_segctor_complete_write(struct nilfs_sc_info *sci)
nilfs_drop_collected_inodes(&sci->sc_gc_inodes);
if (update_sr)
nilfs_commit_gcdat_inode(nilfs);
- } else {
+ } else
nilfs->ns_nongc_ctime = sci->sc_seg_ctime;
- set_nilfs_cond_nongc_write(nilfs);
- wake_up(&nilfs->ns_cleanerd_wq);
- }
sci->sc_nblk_inc += sci->sc_nblk_this_inc;
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 852e0bf..69b6255 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -73,7 +73,6 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev)
nilfs->ns_gc_inodes_h = NULL;
INIT_LIST_HEAD(&nilfs->ns_used_segments);
init_rwsem(&nilfs->ns_segctor_sem);
- init_waitqueue_head(&nilfs->ns_cleanerd_wq);
return nilfs;
}
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 9cd3c11..75da373 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -37,7 +37,6 @@ enum {
THE_NILFS_LOADED, /* Roll-back/roll-forward has done and
the latest checkpoint was loaded */
THE_NILFS_DISCONTINUED, /* 'next' pointer chain has broken */
- THE_NILFS_COND_NONGC_WRITE, /* Condition to wake up cleanerd */
};
/**
@@ -74,7 +73,6 @@ enum {
* @ns_gc_dat: shadow inode of the DAT file inode for GC
* @ns_gc_inodes: dummy inodes to keep live blocks
* @ns_gc_inodes_h: hash list to keep dummy inode holding live blocks
- * @ns_cleanerd_wq: wait queue for cleanerd
* @ns_blocksize_bits: bit length of block size
* @ns_nsegments: number of segments in filesystem
* @ns_blocks_per_segment: number of blocks per segment
@@ -151,9 +149,6 @@ struct the_nilfs {
struct list_head ns_gc_inodes;
struct hlist_head *ns_gc_inodes_h;
- /* cleanerd */
- wait_queue_head_t ns_cleanerd_wq;
-
/* Disk layout information (static) */
unsigned int ns_blocksize_bits;
unsigned long ns_nsegments;
@@ -186,7 +181,6 @@ static inline int nilfs_##name(struct the_nilfs *nilfs) \
THE_NILFS_FNS(INIT, init)
THE_NILFS_FNS(LOADED, loaded)
THE_NILFS_FNS(DISCONTINUED, discontinued)
-THE_NILFS_FNS(COND_NONGC_WRITE, cond_nongc_write)
void nilfs_set_last_segment(struct the_nilfs *, sector_t, u64, __u64);
struct the_nilfs *alloc_nilfs(struct block_device *);
diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
index e38fad2..b0a6b39 100644
--- a/include/linux/nilfs2_fs.h
+++ b/include/linux/nilfs2_fs.h
@@ -763,18 +763,6 @@ struct nilfs_bdesc {
__u32 bd_level;
};
-#define NILFS_TIMEDWAIT_WRITE_LOCKED 0x1
-#define NILFS_TIMEDWAIT_SEG_WRITE 0x2
-
-/**
- * struct nilfs_wait_cond -
- */
-struct nilfs_wait_cond {
- int wc_cond;
- int wc_flags;
- struct timespec wc_timeout;
-};
-
#define NILFS_IOCTL_IDENT 'n'
#define NILFS_IOCTL_CHANGE_CPMODE \
@@ -795,8 +783,6 @@ struct nilfs_wait_cond {
_IOWR(NILFS_IOCTL_IDENT, 0x87, struct nilfs_argv)
#define NILFS_IOCTL_CLEAN_SEGMENTS \
_IOW(NILFS_IOCTL_IDENT, 0x88, struct nilfs_argv[5])
-#define NILFS_IOCTL_TIMEDWAIT \
- _IOWR(NILFS_IOCTL_IDENT, 0x89, struct nilfs_wait_cond)
#define NILFS_IOCTL_SYNC \
_IOR(NILFS_IOCTL_IDENT, 0x8A, __u64)
#define NILFS_IOCTL_RESIZE \
@@ -827,12 +813,6 @@ struct nilfs_sustat32 {
compat_time_t ss_nongc_ctime;
};
-struct nilfs_wait_cond32 {
- compat_int_t wc_cond;
- compat_int_t wc_flags;
- struct compat_timespec wc_timeout;
-};
-
#define NILFS_IOCTL32_CHANGE_CPMODE \
_IOW(NILFS_IOCTL_IDENT, 0x80, struct nilfs_cpmode32)
#define NILFS_IOCTL32_GET_CPINFO \
@@ -847,8 +827,6 @@ struct nilfs_wait_cond32 {
_IOWR(NILFS_IOCTL_IDENT, 0x87, struct nilfs_argv32)
#define NILFS_IOCTL32_CLEAN_SEGMENTS \
_IOW(NILFS_IOCTL_IDENT, 0x88, struct nilfs_argv32[5])
-#define NILFS_IOCTL32_TIMEDWAIT \
- _IOWR(NILFS_IOCTL_IDENT, 0x89, struct nilfs_wait_cond32)
#endif /* CONFIG_COMPAT */
#endif /* _LINUX_NILFS_FS_H */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH mmotm 2/6] nilfs2: use fixed sized types for ioctl structures
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 1/6] nilfs2: remove timedwait ioctl command Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 3/6] nilfs2: remove compat ioctl code Ryusuke Konishi
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi
Nilfs ioctl had structures not having fixed sized types such as:
struct nilfs_argv {
void *v_base;
size_t v_nmembs;
size_t v_size;
int v_index;
int v_flags;
};
Further, some of them are wrongly aligned:
e.g.
struct nilfs_cpmode {
__u64 cm_cno;
int cm_mode;
};
The size of wrongly aligned structures varies depending on
architectures, and it breaks the identity of ioctl commands, which
leads to arch dependent errors.
Previously, these are compensated by using compat_ioctl.
This fixes these problems and allows removal of compat ioctl.
Since this will change sizes of those structures, binary compatibility
for the past utilities will once break; new utilities have to be used
instead. However, it would be helpful to avoid platform dependent
problems in the long term.
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/ioctl.c | 11 +++++------
include/linux/nilfs2_fs.h | 23 ++++++++++++++---------
2 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 85a291c..7fbd9fe 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -41,6 +41,7 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
void *, size_t, size_t))
{
void *buf;
+ void __user *base = (void __user *)(unsigned long)argv->v_base;
size_t maxmembs, total, n;
ssize_t nr;
int ret, i;
@@ -64,9 +65,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
n = (argv->v_nmembs - i < maxmembs) ?
argv->v_nmembs - i : maxmembs;
if ((dir & _IOC_WRITE) &&
- copy_from_user(buf,
- (void __user *)argv->v_base + argv->v_size * i,
- argv->v_size * n)) {
+ copy_from_user(buf, base + argv->v_size * i,
+ argv->v_size * n)) {
ret = -EFAULT;
break;
}
@@ -78,9 +78,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs,
break;
}
if ((dir & _IOC_READ) &&
- copy_to_user(
- (void __user *)argv->v_base + argv->v_size * i,
- buf, argv->v_size * nr)) {
+ copy_to_user(base + argv->v_size * i, buf,
+ argv->v_size * nr)) {
ret = -EFAULT;
break;
}
diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
index b0a6b39..8fb64ce 100644
--- a/include/linux/nilfs2_fs.h
+++ b/include/linux/nilfs2_fs.h
@@ -499,6 +499,7 @@ NILFS_CHECKPOINT_FNS(SKETCH, sketch)
/**
* struct nilfs_cpinfo - checkpoint information
* @ci_flags: flags
+ * @ci_pad: padding
* @ci_cno: checkpoint number
* @ci_create: creation timestamp
* @ci_nblk_inc: number of blocks incremented by this checkpoint
@@ -508,6 +509,7 @@ NILFS_CHECKPOINT_FNS(SKETCH, sketch)
*/
struct nilfs_cpinfo {
__u32 ci_flags;
+ __u32 ci_pad;
__u64 ci_cno;
__u64 ci_create;
__u64 ci_nblk_inc;
@@ -668,7 +670,8 @@ enum {
*/
struct nilfs_cpmode {
__u64 cm_cno;
- int cm_mode;
+ __u32 cm_mode;
+ __u32 cm_pad;
};
/**
@@ -676,15 +679,15 @@ struct nilfs_cpmode {
* @v_base:
* @v_nmembs:
* @v_size:
- * @v_index:
* @v_flags:
+ * @v_index:
*/
struct nilfs_argv {
- void *v_base;
- size_t v_nmembs; /* number of members */
- size_t v_size; /* size of members */
- int v_index;
- int v_flags;
+ __u64 v_base;
+ __u32 v_nmembs; /* number of members */
+ __u16 v_size; /* size of members */
+ __u16 v_flags;
+ __u64 v_index;
};
/**
@@ -721,8 +724,8 @@ struct nilfs_sustat {
__u64 ss_nsegs;
__u64 ss_ncleansegs;
__u64 ss_ndirtysegs;
- time_t ss_ctime;
- time_t ss_nongc_ctime;
+ __u64 ss_ctime;
+ __u64 ss_nongc_ctime;
};
/**
@@ -750,6 +753,7 @@ struct nilfs_vdesc {
__u64 vd_blocknr;
__u64 vd_offset;
__u32 vd_flags;
+ __u32 vd_pad;
};
/**
@@ -761,6 +765,7 @@ struct nilfs_bdesc {
__u64 bd_blocknr;
__u64 bd_offset;
__u32 bd_level;
+ __u32 bd_pad;
};
#define NILFS_IOCTL_IDENT 'n'
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH mmotm 3/6] nilfs2: remove compat ioctl code
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 1/6] nilfs2: remove timedwait ioctl command Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 2/6] nilfs2: use fixed sized types for ioctl structures Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl Ryusuke Konishi
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi
This removes compat code from the nilfs ioctls and applies the same
function for both .ioctl and .compat_ioctl file operations.
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/dir.c | 2 +-
fs/nilfs2/file.c | 2 +-
fs/nilfs2/ioctl.c | 228 ---------------------------------------------
fs/nilfs2/nilfs.h | 1 -
include/linux/nilfs2_fs.h | 41 --------
5 files changed, 2 insertions(+), 272 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 1b7e6dd..393316c 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -704,7 +704,7 @@ struct file_operations nilfs_dir_operations = {
.readdir = nilfs_readdir,
.ioctl = nilfs_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = nilfs_compat_ioctl,
+ .compat_ioctl = nilfs_ioctl,
#endif /* CONFIG_COMPAT */
.fsync = nilfs_sync_file,
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index 34c751b..1670038 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -140,7 +140,7 @@ struct file_operations nilfs_file_operations = {
.aio_write = generic_file_aio_write,
.ioctl = nilfs_ioctl,
#ifdef CONFIG_COMPAT
- .compat_ioctl = nilfs_compat_ioctl,
+ .compat_ioctl = nilfs_ioctl,
#endif /* CONFIG_COMPAT */
.mmap = nilfs_file_mmap,
.open = generic_file_open,
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 7fbd9fe..33aff88 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -631,231 +631,3 @@ int nilfs_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
return -ENOTTY;
}
}
-
-/* compat_ioctl */
-#ifdef CONFIG_COMPAT
-#include <linux/compat.h>
-
-static int nilfs_compat_locked_ioctl(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- int ret;
-
- lock_kernel();
- ret = nilfs_ioctl(inode, filp, cmd, arg);
- unlock_kernel();
- return ret;
-}
-
-static int
-nilfs_compat_ioctl_uargv32_to_uargv(struct nilfs_argv32 __user *uargv32,
- struct nilfs_argv __user *uargv)
-{
- compat_uptr_t base;
- compat_size_t nmembs, size;
- compat_int_t index, flags;
-
- if (get_user(base, &uargv32->v_base) ||
- put_user(compat_ptr(base), &uargv->v_base) ||
- get_user(nmembs, &uargv32->v_nmembs) ||
- put_user(nmembs, &uargv->v_nmembs) ||
- get_user(size, &uargv32->v_size) ||
- put_user(size, &uargv->v_size) ||
- get_user(index, &uargv32->v_index) ||
- put_user(index, &uargv->v_index) ||
- get_user(flags, &uargv32->v_flags) ||
- put_user(flags, &uargv->v_flags))
- return -EFAULT;
- return 0;
-}
-
-static int
-nilfs_compat_ioctl_uargv_to_uargv32(struct nilfs_argv __user *uargv,
- struct nilfs_argv32 __user *uargv32)
-{
- size_t nmembs;
-
- if (get_user(nmembs, &uargv->v_nmembs) ||
- put_user(nmembs, &uargv32->v_nmembs))
- return -EFAULT;
- return 0;
-}
-
-static int
-nilfs_compat_ioctl_get_by_argv(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- struct nilfs_argv __user *uargv;
- struct nilfs_argv32 __user *uargv32;
- int ret;
-
- uargv = compat_alloc_user_space(sizeof(struct nilfs_argv));
- uargv32 = compat_ptr(arg);
- ret = nilfs_compat_ioctl_uargv32_to_uargv(uargv32, uargv);
- if (ret < 0)
- return ret;
-
- ret = nilfs_compat_locked_ioctl(inode, filp, cmd, (unsigned long)uargv);
- if (ret < 0)
- return ret;
-
- return nilfs_compat_ioctl_uargv_to_uargv32(uargv, uargv32);
-}
-
-static int
-nilfs_compat_ioctl_change_cpmode(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- struct nilfs_cpmode __user *ucpmode;
- struct nilfs_cpmode32 __user *ucpmode32;
- int mode;
-
- ucpmode = compat_alloc_user_space(sizeof(struct nilfs_cpmode));
- ucpmode32 = compat_ptr(arg);
- if (copy_in_user(&ucpmode->cm_cno, &ucpmode32->cm_cno,
- sizeof(__u64)) ||
- get_user(mode, &ucpmode32->cm_mode) ||
- put_user(mode, &ucpmode->cm_mode))
- return -EFAULT;
-
- return nilfs_compat_locked_ioctl(
- inode, filp, cmd, (unsigned long)ucpmode);
-}
-
-
-static inline int
-nilfs_compat_ioctl_delete_checkpoint(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_locked_ioctl(inode, filp, cmd, arg);
-}
-
-static inline int
-nilfs_compat_ioctl_get_cpinfo(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_ioctl_get_by_argv(inode, filp, cmd, arg);
-}
-
-static inline int
-nilfs_compat_ioctl_get_cpstat(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_locked_ioctl(inode, filp, cmd, arg);
-}
-
-static inline int
-nilfs_compat_ioctl_get_suinfo(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_ioctl_get_by_argv(inode, filp, cmd, arg);
-}
-
-static int
-nilfs_compat_ioctl_get_sustat(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- struct nilfs_sustat __user *usustat;
- struct nilfs_sustat32 __user *usustat32;
- time_t ctime, nongc_ctime;
- int ret;
-
- usustat = compat_alloc_user_space(sizeof(struct nilfs_sustat));
- ret = nilfs_compat_locked_ioctl(inode, filp, cmd,
- (unsigned long)usustat);
- if (ret < 0)
- return ret;
-
- usustat32 = compat_ptr(arg);
- if (copy_in_user(&usustat32->ss_nsegs, &usustat->ss_nsegs,
- sizeof(__u64)) ||
- copy_in_user(&usustat32->ss_ncleansegs, &usustat->ss_ncleansegs,
- sizeof(__u64)) ||
- copy_in_user(&usustat32->ss_ndirtysegs, &usustat->ss_ndirtysegs,
- sizeof(__u64)) ||
- get_user(ctime, &usustat->ss_ctime) ||
- put_user(ctime, &usustat32->ss_ctime) ||
- get_user(nongc_ctime, &usustat->ss_nongc_ctime) ||
- put_user(nongc_ctime, &usustat32->ss_nongc_ctime))
- return -EFAULT;
- return 0;
-}
-
-static inline int
-nilfs_compat_ioctl_get_vinfo(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_ioctl_get_by_argv(inode, filp, cmd, arg);
-}
-
-static inline int
-nilfs_compat_ioctl_get_bdescs(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_ioctl_get_by_argv(inode, filp, cmd, arg);
-}
-
-static int
-nilfs_compat_ioctl_clean_segments(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- struct nilfs_argv __user *uargv;
- struct nilfs_argv32 __user *uargv32;
- int i, ret;
-
- uargv = compat_alloc_user_space(sizeof(struct nilfs_argv) * 5);
- uargv32 = compat_ptr(arg);
- for (i = 0; i < 5; i++) {
- ret = nilfs_compat_ioctl_uargv32_to_uargv(&uargv32[i],
- &uargv[i]);
- if (ret < 0)
- return ret;
- }
- return nilfs_compat_locked_ioctl(
- inode, filp, cmd, (unsigned long)uargv);
-}
-
-static int nilfs_compat_ioctl_sync(struct inode *inode, struct file *filp,
- unsigned int cmd, unsigned long arg)
-{
- return nilfs_compat_locked_ioctl(inode, filp, cmd, arg);
-}
-
-long nilfs_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
-{
- struct inode *inode = filp->f_dentry->d_inode;
-
- switch (cmd) {
- case NILFS_IOCTL32_CHANGE_CPMODE:
- return nilfs_compat_ioctl_change_cpmode(
- inode, filp, NILFS_IOCTL_CHANGE_CPMODE, arg);
- case NILFS_IOCTL_DELETE_CHECKPOINT:
- return nilfs_compat_ioctl_delete_checkpoint(
- inode, filp, cmd, arg);
- case NILFS_IOCTL32_GET_CPINFO:
- return nilfs_compat_ioctl_get_cpinfo(
- inode, filp, NILFS_IOCTL_GET_CPINFO, arg);
- case NILFS_IOCTL_GET_CPSTAT:
- return nilfs_compat_ioctl_get_cpstat(inode, filp, cmd, arg);
- case NILFS_IOCTL32_GET_SUINFO:
- return nilfs_compat_ioctl_get_suinfo(
- inode, filp, NILFS_IOCTL_GET_SUINFO, arg);
- case NILFS_IOCTL32_GET_SUSTAT:
- return nilfs_compat_ioctl_get_sustat(
- inode, filp, NILFS_IOCTL_GET_SUSTAT, arg);
- case NILFS_IOCTL32_GET_VINFO:
- return nilfs_compat_ioctl_get_vinfo(
- inode, filp, NILFS_IOCTL_GET_VINFO, arg);
- case NILFS_IOCTL32_GET_BDESCS:
- return nilfs_compat_ioctl_get_bdescs(
- inode, filp, NILFS_IOCTL_GET_BDESCS, arg);
- case NILFS_IOCTL32_CLEAN_SEGMENTS:
- return nilfs_compat_ioctl_clean_segments(
- inode, filp, NILFS_IOCTL_CLEAN_SEGMENTS, arg);
- case NILFS_IOCTL_SYNC:
- return nilfs_compat_ioctl_sync(inode, filp, cmd, arg);
- default:
- return -ENOIOCTLCMD;
- }
-}
-#endif
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 30c5341..ad17fa9 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -243,7 +243,6 @@ extern int nilfs_sync_file(struct file *, struct dentry *, int);
/* ioctl.c */
int nilfs_ioctl(struct inode *, struct file *, unsigned int, unsigned long);
-long nilfs_compat_ioctl(struct file *, unsigned int, unsigned long);
int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, void __user *);
/* inode.c */
diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
index 8fb64ce..306c446 100644
--- a/include/linux/nilfs2_fs.h
+++ b/include/linux/nilfs2_fs.h
@@ -793,45 +793,4 @@ struct nilfs_bdesc {
#define NILFS_IOCTL_RESIZE \
_IOW(NILFS_IOCTL_IDENT, 0x8B, __u64)
-/* compat_ioctl */
-#ifdef CONFIG_COMPAT
-#include <linux/compat.h>
-
-struct nilfs_cpmode32 {
- __u64 cm_cno;
- compat_int_t cm_mode;
-};
-
-struct nilfs_argv32 {
- compat_caddr_t v_base;
- compat_size_t v_nmembs;
- compat_size_t v_size;
- compat_int_t v_index;
- compat_int_t v_flags;
-};
-
-struct nilfs_sustat32 {
- __u64 ss_nsegs;
- __u64 ss_ncleansegs;
- __u64 ss_ndirtysegs;
- compat_time_t ss_ctime;
- compat_time_t ss_nongc_ctime;
-};
-
-#define NILFS_IOCTL32_CHANGE_CPMODE \
- _IOW(NILFS_IOCTL_IDENT, 0x80, struct nilfs_cpmode32)
-#define NILFS_IOCTL32_GET_CPINFO \
- _IOR(NILFS_IOCTL_IDENT, 0x82, struct nilfs_argv32)
-#define NILFS_IOCTL32_GET_SUINFO \
- _IOR(NILFS_IOCTL_IDENT, 0x84, struct nilfs_argv32)
-#define NILFS_IOCTL32_GET_SUSTAT \
- _IOR(NILFS_IOCTL_IDENT, 0x85, struct nilfs_sustat32)
-#define NILFS_IOCTL32_GET_VINFO \
- _IOWR(NILFS_IOCTL_IDENT, 0x86, struct nilfs_argv32)
-#define NILFS_IOCTL32_GET_BDESCS \
- _IOWR(NILFS_IOCTL_IDENT, 0x87, struct nilfs_argv32)
-#define NILFS_IOCTL32_CLEAN_SEGMENTS \
- _IOW(NILFS_IOCTL_IDENT, 0x88, struct nilfs_argv32[5])
-#endif /* CONFIG_COMPAT */
-
#endif /* _LINUX_NILFS_FS_H */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
` (2 preceding siblings ...)
2009-02-17 7:42 ` [PATCH mmotm 3/6] nilfs2: remove compat ioctl code Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 11:26 ` Pekka Enberg
2009-02-17 7:42 ` [PATCH mmotm 5/6] nilfs2: extend nilfs_sustat ioctl struct Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 6/6] nilfs2: replace BUG_ON and BUG calls triggerable from ioctl Ryusuke Konishi
5 siblings, 1 reply; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi, Pekka Enberg
Pekka Enberg suggested converting ->ioctl operations to use
->unlocked_ioctl to avoid BKL.
The conversion was verified to be safe, so I will take it on this
occasion.
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/dir.c | 2 +-
fs/nilfs2/file.c | 2 +-
fs/nilfs2/ioctl.c | 4 ++--
fs/nilfs2/nilfs.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 393316c..54100ac 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -702,7 +702,7 @@ struct file_operations nilfs_dir_operations = {
.llseek = generic_file_llseek,
.read = generic_read_dir,
.readdir = nilfs_readdir,
- .ioctl = nilfs_ioctl,
+ .unlocked_ioctl = nilfs_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = nilfs_ioctl,
#endif /* CONFIG_COMPAT */
diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
index 1670038..df74577 100644
--- a/fs/nilfs2/file.c
+++ b/fs/nilfs2/file.c
@@ -138,7 +138,7 @@ struct file_operations nilfs_file_operations = {
.write = do_sync_write,
.aio_read = generic_file_aio_read,
.aio_write = generic_file_aio_write,
- .ioctl = nilfs_ioctl,
+ .unlocked_ioctl = nilfs_ioctl,
#ifdef CONFIG_COMPAT
.compat_ioctl = nilfs_ioctl,
#endif /* CONFIG_COMPAT */
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index 33aff88..cfb2789 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -600,9 +600,9 @@ static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
return 0;
}
-int nilfs_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
- unsigned long arg)
+long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
+ struct inode *inode = filp->f_dentry->d_inode;
void __user *argp = (void * __user *)arg;
switch (cmd) {
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index ad17fa9..f865ecb 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -242,7 +242,7 @@ extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
extern int nilfs_sync_file(struct file *, struct dentry *, int);
/* ioctl.c */
-int nilfs_ioctl(struct inode *, struct file *, unsigned int, unsigned long);
+long nilfs_ioctl(struct file *, unsigned int, unsigned long);
int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, void __user *);
/* inode.c */
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl
2009-02-17 7:42 ` [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl Ryusuke Konishi
@ 2009-02-17 11:26 ` Pekka Enberg
2009-02-17 15:43 ` Ryusuke Konishi
0 siblings, 1 reply; 11+ messages in thread
From: Pekka Enberg @ 2009-02-17 11:26 UTC (permalink / raw)
To: Ryusuke Konishi; +Cc: Andrew Morton, linux-fsdevel, linux-kernel
Hi Ryusuke,
On Tue, 2009-02-17 at 16:42 +0900, Ryusuke Konishi wrote:
> Pekka Enberg suggested converting ->ioctl operations to use
> ->unlocked_ioctl to avoid BKL.
>
> The conversion was verified to be safe, so I will take it on this
> occasion.
>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> ---
> fs/nilfs2/dir.c | 2 +-
> fs/nilfs2/file.c | 2 +-
> fs/nilfs2/ioctl.c | 4 ++--
> fs/nilfs2/nilfs.h | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> index 393316c..54100ac 100644
> --- a/fs/nilfs2/dir.c
> +++ b/fs/nilfs2/dir.c
> @@ -702,7 +702,7 @@ struct file_operations nilfs_dir_operations = {
> .llseek = generic_file_llseek,
> .read = generic_read_dir,
> .readdir = nilfs_readdir,
> - .ioctl = nilfs_ioctl,
> + .unlocked_ioctl = nilfs_ioctl,
> #ifdef CONFIG_COMPAT
> .compat_ioctl = nilfs_ioctl,
> #endif /* CONFIG_COMPAT */
> diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
> index 1670038..df74577 100644
> --- a/fs/nilfs2/file.c
> +++ b/fs/nilfs2/file.c
> @@ -138,7 +138,7 @@ struct file_operations nilfs_file_operations = {
> .write = do_sync_write,
> .aio_read = generic_file_aio_read,
> .aio_write = generic_file_aio_write,
> - .ioctl = nilfs_ioctl,
> + .unlocked_ioctl = nilfs_ioctl,
> #ifdef CONFIG_COMPAT
> .compat_ioctl = nilfs_ioctl,
> #endif /* CONFIG_COMPAT */
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index 33aff88..cfb2789 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -600,9 +600,9 @@ static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
> return 0;
> }
>
> -int nilfs_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
> - unsigned long arg)
> +long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> {
> + struct inode *inode = filp->f_dentry->d_inode;
> void __user *argp = (void * __user *)arg;
>
> switch (cmd) {
> diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> index ad17fa9..f865ecb 100644
> --- a/fs/nilfs2/nilfs.h
> +++ b/fs/nilfs2/nilfs.h
> @@ -242,7 +242,7 @@ extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
> extern int nilfs_sync_file(struct file *, struct dentry *, int);
>
> /* ioctl.c */
> -int nilfs_ioctl(struct inode *, struct file *, unsigned int, unsigned long);
> +long nilfs_ioctl(struct file *, unsigned int, unsigned long);
> int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, void __user *);
>
> /* inode.c */
Maybe I'm looking at an old version of nilfs but don't you need to
remove nilfs_compat_locked_ioctl() and the lock_kernel()/unlock_kernel()
from nilfs_ioctl_timedwait() as well?
Pekka
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl
2009-02-17 11:26 ` Pekka Enberg
@ 2009-02-17 15:43 ` Ryusuke Konishi
2009-02-17 15:49 ` Pekka Enberg
0 siblings, 1 reply; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 15:43 UTC (permalink / raw)
To: penberg; +Cc: konishi.ryusuke, akpm, linux-fsdevel, linux-kernel
Hi Pekka,
On Tue, 17 Feb 2009 13:26:13 +0200, Pekka Enberg wrote:
> Hi Ryusuke,
>
> On Tue, 2009-02-17 at 16:42 +0900, Ryusuke Konishi wrote:
> > Pekka Enberg suggested converting ->ioctl operations to use
> > ->unlocked_ioctl to avoid BKL.
> >
> > The conversion was verified to be safe, so I will take it on this
> > occasion.
> >
> > Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> > Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
> > ---
> > fs/nilfs2/dir.c | 2 +-
> > fs/nilfs2/file.c | 2 +-
> > fs/nilfs2/ioctl.c | 4 ++--
> > fs/nilfs2/nilfs.h | 2 +-
> > 4 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
> > index 393316c..54100ac 100644
> > --- a/fs/nilfs2/dir.c
> > +++ b/fs/nilfs2/dir.c
> > @@ -702,7 +702,7 @@ struct file_operations nilfs_dir_operations = {
> > .llseek = generic_file_llseek,
> > .read = generic_read_dir,
> > .readdir = nilfs_readdir,
> > - .ioctl = nilfs_ioctl,
> > + .unlocked_ioctl = nilfs_ioctl,
> > #ifdef CONFIG_COMPAT
> > .compat_ioctl = nilfs_ioctl,
> > #endif /* CONFIG_COMPAT */
> > diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
> > index 1670038..df74577 100644
> > --- a/fs/nilfs2/file.c
> > +++ b/fs/nilfs2/file.c
> > @@ -138,7 +138,7 @@ struct file_operations nilfs_file_operations = {
> > .write = do_sync_write,
> > .aio_read = generic_file_aio_read,
> > .aio_write = generic_file_aio_write,
> > - .ioctl = nilfs_ioctl,
> > + .unlocked_ioctl = nilfs_ioctl,
> > #ifdef CONFIG_COMPAT
> > .compat_ioctl = nilfs_ioctl,
> > #endif /* CONFIG_COMPAT */
> > diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> > index 33aff88..cfb2789 100644
> > --- a/fs/nilfs2/ioctl.c
> > +++ b/fs/nilfs2/ioctl.c
> > @@ -600,9 +600,9 @@ static int nilfs_ioctl_sync(struct inode *inode, struct file *filp,
> > return 0;
> > }
> >
> > -int nilfs_ioctl(struct inode *inode, struct file *filp, unsigned int cmd,
> > - unsigned long arg)
> > +long nilfs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> > {
> > + struct inode *inode = filp->f_dentry->d_inode;
> > void __user *argp = (void * __user *)arg;
> >
> > switch (cmd) {
> > diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
> > index ad17fa9..f865ecb 100644
> > --- a/fs/nilfs2/nilfs.h
> > +++ b/fs/nilfs2/nilfs.h
> > @@ -242,7 +242,7 @@ extern void nilfs_set_link(struct inode *, struct nilfs_dir_entry *,
> > extern int nilfs_sync_file(struct file *, struct dentry *, int);
> >
> > /* ioctl.c */
> > -int nilfs_ioctl(struct inode *, struct file *, unsigned int, unsigned long);
> > +long nilfs_ioctl(struct file *, unsigned int, unsigned long);
> > int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *, void __user *);
> >
> > /* inode.c */
>
> Maybe I'm looking at an old version of nilfs but don't you need to
> remove nilfs_compat_locked_ioctl() and the lock_kernel()/unlock_kernel()
> from nilfs_ioctl_timedwait() as well?
>
> Pekka
That's right, but the nilfs_ioctl_timedwait() itself was removed by
the first patch of this patch set. And, the compat ioctl was removed
in the third patch. So they don't appear in this patch.
Cheers,
Ryusuke
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH mmotm 5/6] nilfs2: extend nilfs_sustat ioctl struct
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
` (3 preceding siblings ...)
2009-02-17 7:42 ` [PATCH mmotm 4/6] nilfs2: use unlocked_ioctl Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 7:42 ` [PATCH mmotm 6/6] nilfs2: replace BUG_ON and BUG calls triggerable from ioctl Ryusuke Konishi
5 siblings, 0 replies; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi
This adds a new argument to the nilfs_sustat structure.
The extended field allows to delete volatile active state of segments,
which was needed to protect freshly-created segments from garbage
collection but has confused code dealing with segments. This
extension alleviates the mess and gives room for further
simplifications.
The volatile active flag is not persistent, so it's eliminable on this
occasion without affecting compatibility other than the ioctl change.
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/recovery.c | 32 +++++++++++++++++---------------
fs/nilfs2/segment.c | 39 +++++++++------------------------------
fs/nilfs2/sufile.c | 8 ++++++--
fs/nilfs2/super.c | 4 +++-
fs/nilfs2/the_nilfs.c | 18 ------------------
fs/nilfs2/the_nilfs.h | 5 ++---
include/linux/nilfs2_fs.h | 10 ++++------
7 files changed, 41 insertions(+), 75 deletions(-)
diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index 877dc1b..a4253f3 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -416,6 +416,7 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
struct nilfs_segment_entry *ent, *n;
struct inode *sufile = nilfs->ns_sufile;
__u64 segnum[4];
+ time_t mtime;
int err;
int i;
@@ -442,9 +443,9 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
/*
* Collecting segments written after the latest super root.
- * These are marked volatile active, and won't be reallocated in
- * the next construction.
+ * These are marked dirty to avoid being reallocated in the next write.
*/
+ mtime = get_seconds();
list_for_each_entry_safe(ent, n, head, list) {
if (ent->segnum == segnum[0]) {
list_del(&ent->list);
@@ -454,17 +455,16 @@ static int nilfs_prepare_segment_for_recovery(struct the_nilfs *nilfs,
err = nilfs_open_segment_entry(ent, sufile);
if (unlikely(err))
goto failed;
- if (nilfs_segment_usage_clean(ent->raw_su)) {
- nilfs_segment_usage_set_volatile_active(ent->raw_su);
- /* Keep it open */
- } else {
- /* Removing duplicated entries */
- list_del(&ent->list);
- nilfs_close_segment_entry(ent, sufile);
- nilfs_free_segment_entry(ent);
+ if (!nilfs_segment_usage_dirty(ent->raw_su)) {
+ /* make the segment garbage */
+ ent->raw_su->su_nblocks = cpu_to_le32(0);
+ ent->raw_su->su_lastmod = cpu_to_le32(mtime);
+ nilfs_segment_usage_set_dirty(ent->raw_su);
}
+ list_del(&ent->list);
+ nilfs_close_segment_entry(ent, sufile);
+ nilfs_free_segment_entry(ent);
}
- list_splice_init(head, nilfs->ns_used_segments.prev);
/*
* The segment having the latest super root is active, and
@@ -882,10 +882,12 @@ int nilfs_search_super_root(struct the_nilfs *nilfs, struct nilfs_sb_info *sbi,
if (scan_newer)
ri->ri_need_recovery = NILFS_RECOVERY_SR_UPDATED;
- else if (nilfs->ns_mount_state & NILFS_VALID_FS)
- goto super_root_found;
-
- scan_newer = 1;
+ else {
+ nilfs->ns_prot_seq = ssi.seg_seq;
+ if (nilfs->ns_mount_state & NILFS_VALID_FS)
+ goto super_root_found;
+ scan_newer = 1;
+ }
/* reset region for roll-forward */
pseg_start += ssi.nblocks;
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 5db12d7..24d0fbd 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -2229,13 +2229,6 @@ static void nilfs_segctor_reactivate_segments(struct nilfs_sc_info *sci,
nilfs_segment_usage_set_active(ent->raw_su);
nilfs_close_segment_entry(ent, sufile);
}
-
- down_write(&nilfs->ns_sem);
- head = &nilfs->ns_used_segments;
- list_for_each_entry(ent, head, list) {
- nilfs_segment_usage_set_volatile_active(ent->raw_su);
- }
- up_write(&nilfs->ns_sem);
}
static int nilfs_segctor_deactivate_segments(struct nilfs_sc_info *sci,
@@ -2244,7 +2237,6 @@ static int nilfs_segctor_deactivate_segments(struct nilfs_sc_info *sci,
struct nilfs_segment_buffer *segbuf, *last;
struct nilfs_segment_entry *ent;
struct inode *sufile = nilfs->ns_sufile;
- struct list_head *head;
int err;
last = NILFS_LAST_SEGBUF(&sci->sc_segbufs);
@@ -2265,22 +2257,13 @@ static int nilfs_segctor_deactivate_segments(struct nilfs_sc_info *sci,
BUG_ON(!buffer_dirty(ent->bh_su));
}
- head = &sci->sc_active_segments;
- list_for_each_entry(ent, head, list) {
+ list_for_each_entry(ent, &sci->sc_active_segments, list) {
err = nilfs_open_segment_entry(ent, sufile);
if (unlikely(err))
goto failed;
nilfs_segment_usage_clear_active(ent->raw_su);
BUG_ON(!buffer_dirty(ent->bh_su));
}
-
- down_write(&nilfs->ns_sem);
- head = &nilfs->ns_used_segments;
- list_for_each_entry(ent, head, list) {
- /* clear volatile active for segments of older generations */
- nilfs_segment_usage_clear_volatile_active(ent->raw_su);
- }
- up_write(&nilfs->ns_sem);
return 0;
failed:
@@ -2304,19 +2287,15 @@ static void nilfs_segctor_bead_completed_segments(struct nilfs_sc_info *sci)
}
}
-static void
-__nilfs_segctor_commit_deactivate_segments(struct nilfs_sc_info *sci,
- struct the_nilfs *nilfs)
-
+static void nilfs_segctor_commit_deactivate_segments(struct nilfs_sc_info *sci,
+ struct the_nilfs *nilfs)
{
- struct nilfs_segment_entry *ent;
-
- list_splice_init(&sci->sc_active_segments,
- nilfs->ns_used_segments.prev);
+ struct nilfs_segment_entry *ent, *n;
- list_for_each_entry(ent, &nilfs->ns_used_segments, list) {
- nilfs_segment_usage_set_volatile_active(ent->raw_su);
- /* These segments are kept open */
+ list_for_each_entry_safe(ent, n, &sci->sc_active_segments, list) {
+ list_del(&ent->list);
+ nilfs_close_segment_entry(ent, nilfs->ns_sufile);
+ nilfs_free_segment_entry(ent);
}
}
@@ -2405,8 +2384,8 @@ static int nilfs_segctor_do_construct(struct nilfs_sc_info *sci, int mode)
if (has_sr) {
down_write(&nilfs->ns_sem);
nilfs_update_last_segment(sbi, 1);
- __nilfs_segctor_commit_deactivate_segments(sci, nilfs);
up_write(&nilfs->ns_sem);
+ nilfs_segctor_commit_deactivate_segments(sci, nilfs);
nilfs_segctor_commit_free_segments(sci);
nilfs_segctor_clear_metadata_dirty(sci);
}
diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
index b3674a8..cc714c7 100644
--- a/fs/nilfs2/sufile.c
+++ b/fs/nilfs2/sufile.c
@@ -446,6 +446,7 @@ int nilfs_sufile_get_stat(struct inode *sufile, struct nilfs_sustat *sustat)
{
struct buffer_head *header_bh;
struct nilfs_sufile_header *header;
+ struct the_nilfs *nilfs = NILFS_MDT(sufile)->mi_nilfs;
void *kaddr;
int ret;
@@ -460,8 +461,11 @@ int nilfs_sufile_get_stat(struct inode *sufile, struct nilfs_sustat *sustat)
sustat->ss_nsegs = nilfs_sufile_get_nsegments(sufile);
sustat->ss_ncleansegs = le64_to_cpu(header->sh_ncleansegs);
sustat->ss_ndirtysegs = le64_to_cpu(header->sh_ndirtysegs);
- sustat->ss_ctime = NILFS_MDT(sufile)->mi_nilfs->ns_ctime;
- sustat->ss_nongc_ctime = NILFS_MDT(sufile)->mi_nilfs->ns_nongc_ctime;
+ sustat->ss_ctime = nilfs->ns_ctime;
+ sustat->ss_nongc_ctime = nilfs->ns_nongc_ctime;
+ spin_lock(&nilfs->ns_last_segment_lock);
+ sustat->ss_prot_seq = nilfs->ns_prot_seq;
+ spin_unlock(&nilfs->ns_last_segment_lock);
kunmap_atomic(kaddr, KM_USER0);
brelse(header_bh);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 268b563..2f0e9f7 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -262,8 +262,10 @@ static int nilfs_sync_super(struct nilfs_sb_info *sbi)
printk(KERN_ERR
"NILFS: unable to write superblock (err=%d)\n", err);
else {
- nilfs_dispose_used_segments(nilfs);
clear_nilfs_discontinued(nilfs);
+ spin_lock(&nilfs->ns_last_segment_lock);
+ nilfs->ns_prot_seq = le64_to_cpu(nilfs->ns_sbp->s_last_seq);
+ spin_unlock(&nilfs->ns_last_segment_lock);
}
return err;
diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c
index 69b6255..661ab76 100644
--- a/fs/nilfs2/the_nilfs.c
+++ b/fs/nilfs2/the_nilfs.c
@@ -71,7 +71,6 @@ struct the_nilfs *alloc_nilfs(struct block_device *bdev)
INIT_LIST_HEAD(&nilfs->ns_supers);
spin_lock_init(&nilfs->ns_last_segment_lock);
nilfs->ns_gc_inodes_h = NULL;
- INIT_LIST_HEAD(&nilfs->ns_used_segments);
init_rwsem(&nilfs->ns_segctor_sem);
return nilfs;
@@ -95,7 +94,6 @@ void put_nilfs(struct the_nilfs *nilfs)
*/
might_sleep();
if (nilfs_loaded(nilfs)) {
- nilfs_dispose_used_segments(nilfs);
nilfs_mdt_clear(nilfs->ns_sufile);
nilfs_mdt_destroy(nilfs->ns_sufile);
nilfs_mdt_clear(nilfs->ns_cpfile);
@@ -463,22 +461,6 @@ int nilfs_count_free_blocks(struct the_nilfs *nilfs, sector_t *nblocks)
return err;
}
-void nilfs_dispose_used_segments(struct the_nilfs *nilfs)
-{
- struct nilfs_segment_entry *ent, *n;
-
- /* nilfs->sem must be locked by the caller. */
- if (!nilfs_loaded(nilfs))
- return;
-
- list_for_each_entry_safe(ent, n, &nilfs->ns_used_segments, list) {
- list_del_init(&ent->list);
- nilfs_segment_usage_clear_volatile_active(ent->raw_su);
- nilfs_close_segment_entry(ent, nilfs->ns_sufile);
- nilfs_free_segment_entry(ent);
- }
-}
-
int nilfs_near_disk_full(struct the_nilfs *nilfs)
{
struct inode *sufile = nilfs->ns_sufile;
diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
index 75da373..af566e7 100644
--- a/fs/nilfs2/the_nilfs.h
+++ b/fs/nilfs2/the_nilfs.h
@@ -51,7 +51,6 @@ enum {
* @ns_writer_refcount: number of referrers on ns_writer
* @ns_sbh: buffer head of the on-disk super block
* @ns_sbp: pointer to the super block data
- * @ns_used_segments: list of full segments in volatile active state
* @ns_supers: list of nilfs super block structs
* @ns_seg_seq: segment sequence counter
* @ns_segnum: index number of the latest full segment.
@@ -65,6 +64,7 @@ enum {
* @ns_last_pseg: start block number of the latest segment
* @ns_last_seq: sequence value of the latest segment
* @ns_last_cno: checkpoint number of the latest segment
+ * @ns_prot_seq: least sequence number of segments which must not be reclaimed
* @ns_free_segments_count: counter of free segments
* @ns_segctor_sem: segment constructor semaphore
* @ns_dat: DAT file inode
@@ -103,7 +103,6 @@ struct the_nilfs {
*/
struct buffer_head *ns_sbh;
struct nilfs_super_block *ns_sbp;
- struct list_head ns_used_segments;
unsigned ns_mount_state;
struct list_head ns_supers;
@@ -132,6 +131,7 @@ struct the_nilfs {
sector_t ns_last_pseg;
u64 ns_last_seq;
__u64 ns_last_cno;
+ u64 ns_prot_seq;
unsigned long ns_free_segments_count;
struct rw_semaphore ns_segctor_sem;
@@ -188,7 +188,6 @@ void put_nilfs(struct the_nilfs *);
int init_nilfs(struct the_nilfs *, struct nilfs_sb_info *, char *);
int load_nilfs(struct the_nilfs *, struct nilfs_sb_info *);
int nilfs_count_free_blocks(struct the_nilfs *, sector_t *);
-void nilfs_dispose_used_segments(struct the_nilfs *);
int nilfs_checkpoint_is_mounted(struct the_nilfs *, __u64, int);
int nilfs_near_disk_full(struct the_nilfs *);
diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
index 306c446..aa93f0e 100644
--- a/include/linux/nilfs2_fs.h
+++ b/include/linux/nilfs2_fs.h
@@ -565,8 +565,6 @@ enum {
NILFS_SEGMENT_USAGE_DIRTY,
NILFS_SEGMENT_USAGE_ERROR,
- /* on-memory only */
- NILFS_SEGMENT_USAGE_VOLATILE_ACTIVE,
/* ... */
};
@@ -594,7 +592,6 @@ nilfs_segment_usage_##name(const struct nilfs_segment_usage *su) \
NILFS_SEGMENT_USAGE_FNS(ACTIVE, active)
NILFS_SEGMENT_USAGE_FNS(DIRTY, dirty)
NILFS_SEGMENT_USAGE_FNS(ERROR, error)
-NILFS_SEGMENT_USAGE_FNS(VOLATILE_ACTIVE, volatile_active)
static inline void
nilfs_segment_usage_set_clean(struct nilfs_segment_usage *su)
@@ -650,7 +647,6 @@ nilfs_suinfo_##name(const struct nilfs_suinfo *si) \
NILFS_SUINFO_FNS(ACTIVE, active)
NILFS_SUINFO_FNS(DIRTY, dirty)
NILFS_SUINFO_FNS(ERROR, error)
-NILFS_SUINFO_FNS(VOLATILE_ACTIVE, volatile_active)
static inline int nilfs_suinfo_clean(const struct nilfs_suinfo *si)
{
@@ -717,8 +713,9 @@ struct nilfs_cpstat {
* @ss_nsegs: number of segments
* @ss_ncleansegs: number of clean segments
* @ss_ndirtysegs: number of dirty segments
- * @ss_ctime:
- * @ss_nongc_ctime:
+ * @ss_ctime: creation time of the last segment
+ * @ss_nongc_ctime: creation time of the last segment not for GC
+ * @ss_prot_seq: least sequence number of segments which must not be reclaimed
*/
struct nilfs_sustat {
__u64 ss_nsegs;
@@ -726,6 +723,7 @@ struct nilfs_sustat {
__u64 ss_ndirtysegs;
__u64 ss_ctime;
__u64 ss_nongc_ctime;
+ __u64 ss_prot_seq;
};
/**
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH mmotm 6/6] nilfs2: replace BUG_ON and BUG calls triggerable from ioctl
2009-02-17 7:42 [PATCH mmotm 0/6] nilfs2 ioctl cleanups (change ioctl API) Ryusuke Konishi
` (4 preceding siblings ...)
2009-02-17 7:42 ` [PATCH mmotm 5/6] nilfs2: extend nilfs_sustat ioctl struct Ryusuke Konishi
@ 2009-02-17 7:42 ` Ryusuke Konishi
2009-02-17 11:28 ` Pekka Enberg
5 siblings, 1 reply; 11+ messages in thread
From: Ryusuke Konishi @ 2009-02-17 7:42 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, linux-kernel, Ryusuke Konishi, Pekka Enberg
Pekka Enberg advised me:
> It would be nice if BUG(), BUG_ON(), and panic() calls would be
> converted to proper error handling using WARN_ON() calls. The BUG()
> call in nilfs_cpfile_delete_checkpoints(), for example, looks to be
> triggerable from user-space via the ioctl() system call.
This will follow the comment and keep them to a minimum.
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
---
fs/nilfs2/btree.c | 27 ++++++-----------
fs/nilfs2/cpfile.c | 38 +++++++++++++-----------
fs/nilfs2/dat.c | 15 +++++----
fs/nilfs2/direct.c | 13 ++++++--
fs/nilfs2/inode.c | 19 +++---------
fs/nilfs2/ioctl.c | 63 +++++++++++++++++++++++++++-------------
fs/nilfs2/mdt.c | 4 +--
fs/nilfs2/nilfs.h | 1 -
fs/nilfs2/page.c | 10 ++----
fs/nilfs2/recovery.c | 3 --
fs/nilfs2/segment.c | 78 +++++++++++++++-----------------------------------
fs/nilfs2/sufile.c | 25 +++++++++------
fs/nilfs2/super.c | 8 +++--
13 files changed, 144 insertions(+), 160 deletions(-)
diff --git a/fs/nilfs2/btree.c b/fs/nilfs2/btree.c
index 53f0d4c..6b37a27 100644
--- a/fs/nilfs2/btree.c
+++ b/fs/nilfs2/btree.c
@@ -425,7 +425,6 @@ static int nilfs_btree_node_lookup(const struct nilfs_btree *btree,
index++;
out:
- BUG_ON(indexp == NULL);
*indexp = index;
return s == 0;
@@ -477,8 +476,6 @@ static int nilfs_btree_do_lookup(const struct nilfs_btree *btree,
__u64 ptr;
int level, index, found, ret;
- BUG_ON(minlevel <= NILFS_BTREE_LEVEL_DATA);
-
node = nilfs_btree_get_root(btree);
level = nilfs_btree_node_get_level(btree, node);
if ((level < minlevel) ||
@@ -505,7 +502,7 @@ static int nilfs_btree_do_lookup(const struct nilfs_btree *btree,
if (index < nilfs_btree_node_nchildren_max(btree, node))
ptr = nilfs_btree_node_get_ptr(btree, node, index);
else {
- BUG_ON(found || level != NILFS_BTREE_LEVEL_NODE_MIN);
+ WARN_ON(found || level != NILFS_BTREE_LEVEL_NODE_MIN);
/* insert */
ptr = NILFS_BMAP_INVALID_PTR;
}
@@ -1366,7 +1363,7 @@ static int nilfs_btree_prepare_delete(struct nilfs_btree *btree,
} else {
/* no siblings */
/* the only child of the root node */
- BUG_ON(level != nilfs_btree_height(btree) - 2);
+ WARN_ON(level != nilfs_btree_height(btree) - 2);
if (nilfs_btree_node_get_nchildren(btree, node) - 1 <=
NILFS_BTREE_ROOT_NCHILDREN_MAX) {
path[level].bp_op = nilfs_btree_shrink;
@@ -1543,7 +1540,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *bmap,
break;
case 3:
nchildren = nilfs_btree_node_get_nchildren(btree, root);
- BUG_ON(nchildren > 1);
+ WARN_ON(nchildren > 1);
ptr = nilfs_btree_node_get_ptr(btree, root, nchildren - 1);
ret = nilfs_bmap_get_block(bmap, ptr, &bh);
if (ret < 0)
@@ -1552,7 +1549,7 @@ static int nilfs_btree_gather_data(struct nilfs_bmap *bmap,
break;
default:
node = NULL;
- BUG();
+ return -EINVAL;
}
nchildren = nilfs_btree_node_get_nchildren(btree, node);
@@ -1833,14 +1830,13 @@ static int nilfs_btree_prepare_propagate_v(struct nilfs_btree *btree,
while ((++level < nilfs_btree_height(btree) - 1) &&
!buffer_dirty(path[level].bp_bh)) {
- BUG_ON(buffer_nilfs_volatile(path[level].bp_bh));
+ WARN_ON(buffer_nilfs_volatile(path[level].bp_bh));
ret = nilfs_btree_prepare_update_v(btree, path, level);
if (ret < 0)
goto out;
}
/* success */
- BUG_ON(maxlevelp == NULL);
*maxlevelp = level - 1;
return 0;
@@ -1909,7 +1905,7 @@ static int nilfs_btree_propagate(const struct nilfs_bmap *bmap,
__u64 key;
int level, ret;
- BUG_ON(!buffer_dirty(bh));
+ WARN_ON(!buffer_dirty(bh));
btree = (struct nilfs_btree *)bmap;
path = nilfs_btree_alloc_path(btree);
@@ -1928,12 +1924,9 @@ static int nilfs_btree_propagate(const struct nilfs_bmap *bmap,
ret = nilfs_btree_do_lookup(btree, path, key, NULL, level + 1);
if (ret < 0) {
- /* BUG_ON(ret == -ENOENT); */
- if (ret == -ENOENT) {
+ if (unlikely(ret == -ENOENT))
printk(KERN_CRIT "%s: key = %llu, level == %d\n",
__func__, (unsigned long long)key, level);
- BUG();
- }
goto out;
}
@@ -2117,7 +2110,7 @@ static int nilfs_btree_assign(struct nilfs_bmap *bmap,
ret = nilfs_btree_do_lookup(btree, path, key, NULL, level + 1);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
goto out;
}
@@ -2175,12 +2168,12 @@ static int nilfs_btree_mark(struct nilfs_bmap *bmap, __u64 key, int level)
ret = nilfs_btree_do_lookup(btree, path, key, &ptr, level + 1);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
goto out;
}
ret = nilfs_bmap_get_block(&btree->bt_bmap, ptr, &bh);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
goto out;
}
diff --git a/fs/nilfs2/cpfile.c b/fs/nilfs2/cpfile.c
index eb9b2d8..2e3b066 100644
--- a/fs/nilfs2/cpfile.c
+++ b/fs/nilfs2/cpfile.c
@@ -40,10 +40,7 @@ nilfs_cpfile_checkpoints_per_block(const struct inode *cpfile)
static unsigned long
nilfs_cpfile_get_blkoff(const struct inode *cpfile, __u64 cno)
{
- __u64 tcno;
-
- BUG_ON(cno == 0); /* checkpoint number 0 is invalid */
- tcno = cno + NILFS_MDT(cpfile)->mi_first_entry_offset - 1;
+ __u64 tcno = cno + NILFS_MDT(cpfile)->mi_first_entry_offset - 1;
do_div(tcno, nilfs_cpfile_checkpoints_per_block(cpfile));
return (unsigned long)tcno;
}
@@ -96,7 +93,7 @@ nilfs_cpfile_block_sub_valid_checkpoints(const struct inode *cpfile,
struct nilfs_checkpoint *cp = kaddr + bh_offset(bh);
unsigned int count;
- BUG_ON(le32_to_cpu(cp->cp_checkpoints_count) < n);
+ WARN_ON(le32_to_cpu(cp->cp_checkpoints_count) < n);
count = le32_to_cpu(cp->cp_checkpoints_count) - n;
cp->cp_checkpoints_count = cpu_to_le32(count);
return count;
@@ -178,6 +175,8 @@ static inline int nilfs_cpfile_delete_checkpoint_block(struct inode *cpfile,
* %-ENOMEM - Insufficient amount of memory available.
*
* %-ENOENT - No such checkpoint.
+ *
+ * %-EINVAL - invalid checkpoint.
*/
int nilfs_cpfile_get_checkpoint(struct inode *cpfile,
__u64 cno,
@@ -191,8 +190,9 @@ int nilfs_cpfile_get_checkpoint(struct inode *cpfile,
void *kaddr;
int ret;
- BUG_ON(cno < 1 || cno > nilfs_mdt_cno(cpfile) ||
- (cno < nilfs_mdt_cno(cpfile) && create));
+ if (unlikely(cno < 1 || cno > nilfs_mdt_cno(cpfile) ||
+ (cno < nilfs_mdt_cno(cpfile) && create)))
+ return -EINVAL;
down_write(&NILFS_MDT(cpfile)->mi_sem);
@@ -288,12 +288,11 @@ int nilfs_cpfile_delete_checkpoints(struct inode *cpfile,
unsigned long tnicps;
int ret, ncps, nicps, count, i;
- if ((start == 0) || (start > end)) {
- printk(KERN_CRIT "%s: start = %llu, end = %llu\n",
- __func__,
- (unsigned long long)start,
- (unsigned long long)end);
- BUG();
+ if (unlikely(start == 0 || start > end)) {
+ printk(KERN_ERR "%s: invalid range of checkpoint numbers: "
+ "[%llu, %llu)\n", __func__,
+ (unsigned long long)start, (unsigned long long)end);
+ return -EINVAL;
}
/* cannot delete the latest checkpoint */
@@ -323,7 +322,7 @@ int nilfs_cpfile_delete_checkpoints(struct inode *cpfile,
cpfile, cno, cp_bh, kaddr);
nicps = 0;
for (i = 0; i < ncps; i++, cp = (void *)cp + cpsz) {
- BUG_ON(nilfs_checkpoint_snapshot(cp));
+ WARN_ON(nilfs_checkpoint_snapshot(cp));
if (!nilfs_checkpoint_invalid(cp)) {
nilfs_checkpoint_set_invalid(cp);
nicps++;
@@ -393,6 +392,8 @@ static ssize_t nilfs_cpfile_do_get_cpinfo(struct inode *cpfile, __u64 *cnop,
int n, ret;
int ncps, i;
+ if (cno == 0)
+ return -ENOENT; /* checkpoint number 0 is invalid */
down_read(&NILFS_MDT(cpfile)->mi_sem);
for (n = 0; cno < cur_cno && n < nci; cno += ncps) {
@@ -526,9 +527,6 @@ int nilfs_cpfile_delete_checkpoint(struct inode *cpfile, __u64 cno)
ssize_t nci;
int ret;
- /* checkpoint number 0 is invalid */
- if (cno == 0)
- return -ENOENT;
nci = nilfs_cpfile_do_get_cpinfo(cpfile, &tcno, &ci, 1);
if (nci < 0)
return nci;
@@ -576,6 +574,8 @@ static int nilfs_cpfile_set_snapshot(struct inode *cpfile, __u64 cno)
void *kaddr;
int ret;
+ if (cno == 0)
+ return -ENOENT; /* checkpoint number 0 is invalid */
down_write(&NILFS_MDT(cpfile)->mi_sem);
ret = nilfs_cpfile_get_checkpoint_block(cpfile, cno, 0, &cp_bh);
@@ -692,6 +692,8 @@ static int nilfs_cpfile_clear_snapshot(struct inode *cpfile, __u64 cno)
void *kaddr;
int ret;
+ if (cno == 0)
+ return -ENOENT; /* checkpoint number 0 is invalid */
down_write(&NILFS_MDT(cpfile)->mi_sem);
ret = nilfs_cpfile_get_checkpoint_block(cpfile, cno, 0, &cp_bh);
@@ -807,6 +809,8 @@ int nilfs_cpfile_is_snapshot(struct inode *cpfile, __u64 cno)
void *kaddr;
int ret;
+ if (cno == 0)
+ return -ENOENT; /* checkpoint number 0 is invalid */
down_read(&NILFS_MDT(cpfile)->mi_sem);
ret = nilfs_cpfile_get_checkpoint_block(cpfile, cno, 0, &bh);
diff --git a/fs/nilfs2/dat.c b/fs/nilfs2/dat.c
index 9360920..bb8a581 100644
--- a/fs/nilfs2/dat.c
+++ b/fs/nilfs2/dat.c
@@ -135,7 +135,7 @@ int nilfs_dat_prepare_start(struct inode *dat, struct nilfs_palloc_req *req)
int ret;
ret = nilfs_dat_prepare_entry(dat, req, 0);
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
return ret;
}
@@ -157,7 +157,6 @@ void nilfs_dat_commit_start(struct inode *dat, struct nilfs_palloc_req *req,
(unsigned long long)le64_to_cpu(entry->de_start),
(unsigned long long)le64_to_cpu(entry->de_end),
(unsigned long long)le64_to_cpu(entry->de_blocknr));
- BUG();
}
entry->de_blocknr = cpu_to_le64(blocknr);
kunmap_atomic(kaddr, KM_USER0);
@@ -180,7 +179,7 @@ int nilfs_dat_prepare_end(struct inode *dat, struct nilfs_palloc_req *req)
ret = nilfs_dat_prepare_entry(dat, req, 0);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
return ret;
}
@@ -216,7 +215,7 @@ void nilfs_dat_commit_end(struct inode *dat, struct nilfs_palloc_req *req,
end = start = le64_to_cpu(entry->de_start);
if (!dead) {
end = nilfs_mdt_cno(dat);
- BUG_ON(start > end);
+ WARN_ON(start > end);
}
entry->de_end = cpu_to_le64(end);
blocknr = le64_to_cpu(entry->de_blocknr);
@@ -324,14 +323,16 @@ int nilfs_dat_move(struct inode *dat, __u64 vblocknr, sector_t blocknr)
return ret;
kaddr = kmap_atomic(entry_bh->b_page, KM_USER0);
entry = nilfs_palloc_block_get_entry(dat, vblocknr, entry_bh, kaddr);
- if (entry->de_blocknr == cpu_to_le64(0)) {
+ if (unlikely(entry->de_blocknr == cpu_to_le64(0))) {
printk(KERN_CRIT "%s: vbn = %llu, [%llu, %llu)\n", __func__,
(unsigned long long)vblocknr,
(unsigned long long)le64_to_cpu(entry->de_start),
(unsigned long long)le64_to_cpu(entry->de_end));
- BUG();
+ kunmap_atomic(kaddr, KM_USER0);
+ brelse(entry_bh);
+ return -EINVAL;
}
- BUG_ON(blocknr == 0);
+ WARN_ON(blocknr == 0);
entry->de_blocknr = cpu_to_le64(blocknr);
kunmap_atomic(kaddr, KM_USER0);
diff --git a/fs/nilfs2/direct.c b/fs/nilfs2/direct.c
index e3ec248..c6379e4 100644
--- a/fs/nilfs2/direct.c
+++ b/fs/nilfs2/direct.c
@@ -210,7 +210,6 @@ static int nilfs_direct_last_key(const struct nilfs_bmap *bmap, __u64 *keyp)
if (lastkey == NILFS_DIRECT_KEY_MAX + 1)
return -ENOENT;
- BUG_ON(keyp == NULL);
*keyp = lastkey;
return 0;
@@ -366,9 +365,17 @@ static int nilfs_direct_assign(struct nilfs_bmap *bmap,
direct = (struct nilfs_direct *)bmap;
key = nilfs_bmap_data_get_key(bmap, *bh);
- BUG_ON(key > NILFS_DIRECT_KEY_MAX);
+ if (unlikely(key > NILFS_DIRECT_KEY_MAX)) {
+ printk(KERN_CRIT "%s: invalid key: %llu\n", __func__,
+ (unsigned long long)key);
+ return -EINVAL;
+ }
ptr = nilfs_direct_get_ptr(direct, key);
- BUG_ON(ptr == NILFS_BMAP_INVALID_PTR);
+ if (unlikely(ptr == NILFS_BMAP_INVALID_PTR)) {
+ printk(KERN_CRIT "%s: invalid pointer: %llu\n", __func__,
+ (unsigned long long)ptr);
+ return -EINVAL;
+ }
return direct->d_ops->dop_assign(direct, key, ptr, bh,
blocknr, binfo);
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 4bf1e2c..b6536bb 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -61,12 +61,6 @@ int nilfs_get_block(struct inode *inode, sector_t blkoff,
map_bh(bh_result, inode->i_sb, blknum);
goto out;
}
- if (unlikely(ret == 1)) {
- printk(KERN_ERR "nilfs_get_block: bmap_lookup returns "
- "buffer_head pointer (blkoff=%llu, blknum=%lu)\n",
- (unsigned long long)blkoff, blknum);
- BUG();
- }
/* data block was not found */
if (ret == -ENOENT && create) {
struct nilfs_transaction_info ti;
@@ -85,14 +79,14 @@ int nilfs_get_block(struct inode *inode, sector_t blkoff,
* However, the page having this block must
* be locked in this case.
*/
- printk(KERN_ERR
+ printk(KERN_WARNING
"nilfs_get_block: a race condition "
"while inserting a data block. "
"(inode number=%lu, file block "
"offset=%llu)\n",
inode->i_ino,
(unsigned long long)blkoff);
- BUG();
+ err = 0;
} else if (err == -EINVAL) {
nilfs_error(inode->i_sb, __func__,
"broken bmap (inode=%lu)\n",
@@ -621,7 +615,6 @@ void nilfs_truncate(struct inode *inode)
struct nilfs_transaction_info ti;
struct super_block *sb = inode->i_sb;
struct nilfs_inode_info *ii = NILFS_I(inode);
- int ret;
if (!test_bit(NILFS_I_BMAP, &ii->i_state))
return;
@@ -630,8 +623,7 @@ void nilfs_truncate(struct inode *inode)
blocksize = sb->s_blocksize;
blkoff = (inode->i_size + blocksize - 1) >> sb->s_blocksize_bits;
- ret = nilfs_transaction_begin(sb, &ti, 0);
- BUG_ON(ret);
+ nilfs_transaction_begin(sb, &ti, 0); /* never fails */
block_truncate_page(inode->i_mapping, inode->i_size, nilfs_get_block);
@@ -652,7 +644,6 @@ void nilfs_delete_inode(struct inode *inode)
struct nilfs_transaction_info ti;
struct super_block *sb = inode->i_sb;
struct nilfs_inode_info *ii = NILFS_I(inode);
- int err;
if (unlikely(is_bad_inode(inode))) {
if (inode->i_data.nrpages)
@@ -660,8 +651,8 @@ void nilfs_delete_inode(struct inode *inode)
clear_inode(inode);
return;
}
- err = nilfs_transaction_begin(sb, &ti, 0);
- BUG_ON(err);
+ nilfs_transaction_begin(sb, &ti, 0); /* never fails */
+
if (inode->i_data.nrpages)
truncate_inode_pages(&inode->i_data, 0);
diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
index cfb2789..108d281 100644
--- a/fs/nilfs2/ioctl.c
+++ b/fs/nilfs2/ioctl.c
@@ -489,14 +489,14 @@ nilfs_ioctl_do_mark_blocks_dirty(struct the_nilfs *nilfs, __u64 *posp,
ret = nilfs_mdt_mark_block_dirty(dat,
bdescs[i].bd_offset);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
return ret;
}
} else {
ret = nilfs_bmap_mark(bmap, bdescs[i].bd_offset,
bdescs[i].bd_level);
if (ret < 0) {
- BUG_ON(ret == -ENOENT);
+ WARN_ON(ret == -ENOENT);
return ret;
}
}
@@ -519,7 +519,8 @@ nilfs_ioctl_do_free_segments(struct the_nilfs *nilfs, __u64 *posp, int flags,
struct nilfs_sb_info *sbi = nilfs_get_writer(nilfs);
int ret;
- BUG_ON(!sbi);
+ if (unlikely(!sbi))
+ return -EROFS;
ret = nilfs_segctor_add_segments_to_be_freed(
NILFS_SC(sbi), buf, nmembs);
nilfs_put_writer(nilfs);
@@ -539,6 +540,7 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
void __user *argp)
{
struct nilfs_argv argv[5];
+ const char *msg;
int dir, ret;
if (copy_from_user(argv, argp, sizeof(argv)))
@@ -546,31 +548,50 @@ int nilfs_ioctl_prepare_clean_segments(struct the_nilfs *nilfs,
dir = _IOC_WRITE;
ret = nilfs_ioctl_move_blocks(nilfs, &argv[0], dir);
- if (ret < 0)
- goto out_move_blks;
+ if (ret < 0) {
+ msg = "cannot read source blocks";
+ goto failed;
+ }
ret = nilfs_ioctl_delete_checkpoints(nilfs, &argv[1], dir);
- if (ret < 0)
- goto out_del_cps;
+ if (ret < 0) {
+ /*
+ * can safely abort because checkpoints can be removed
+ * independently.
+ */
+ msg = "cannot delete checkpoints";
+ goto failed;
+ }
ret = nilfs_ioctl_free_vblocknrs(nilfs, &argv[2], dir);
- if (ret < 0)
- goto out_free_vbns;
+ if (ret < 0) {
+ /*
+ * can safely abort because DAT file is updated atomically
+ * using a copy-on-write technique.
+ */
+ msg = "cannot delete virtual blocks from DAT file";
+ goto failed;
+ }
ret = nilfs_ioctl_mark_blocks_dirty(nilfs, &argv[3], dir);
- if (ret < 0)
- goto out_free_vbns;
+ if (ret < 0) {
+ /*
+ * can safely abort because the operation is nondestructive.
+ */
+ msg = "cannot mark copying blocks dirty";
+ goto failed;
+ }
ret = nilfs_ioctl_free_segments(nilfs, &argv[4], dir);
- if (ret < 0)
- goto out_free_segs;
-
+ if (ret < 0) {
+ /*
+ * can safely abort because this operation is atomic.
+ */
+ msg = "cannot set segments to be freed";
+ goto failed;
+ }
return 0;
- out_free_segs:
- BUG(); /* XXX: not implemented yet */
- out_free_vbns:
- BUG();/* XXX: not implemented yet */
- out_del_cps:
- BUG();/* XXX: not implemented yet */
- out_move_blks:
+ failed:
nilfs_remove_all_gcinode(nilfs);
+ printk(KERN_ERR "NILFS: GC failed during preparation: %s: err=%d\n",
+ msg, ret);
return ret;
}
diff --git a/fs/nilfs2/mdt.c b/fs/nilfs2/mdt.c
index e0a632b..47dd815 100644
--- a/fs/nilfs2/mdt.c
+++ b/fs/nilfs2/mdt.c
@@ -154,10 +154,8 @@ nilfs_mdt_submit_block(struct inode *inode, unsigned long blkoff,
ret = -EBUSY;
goto failed_bh;
}
- } else {
- BUG_ON(mode != READ);
+ } else /* mode == READ */
lock_buffer(bh);
- }
if (buffer_uptodate(bh)) {
unlock_buffer(bh);
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index f865ecb..84e747d 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -173,7 +173,6 @@ static inline void nilfs_set_transaction_flag(unsigned int flag)
{
struct nilfs_transaction_info *ti = current->journal_info;
- BUG_ON(!ti);
ti->ti_flags |= flag;
}
diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
index 7b18be8..1bfbba9 100644
--- a/fs/nilfs2/page.c
+++ b/fs/nilfs2/page.c
@@ -417,7 +417,7 @@ repeat:
dpage = find_lock_page(dmap, offset);
if (dpage) {
/* override existing page on the destination cache */
- BUG_ON(PageDirty(dpage));
+ WARN_ON(PageDirty(dpage));
nilfs_copy_page(dpage, page, 0);
unlock_page(dpage);
page_cache_release(dpage);
@@ -427,17 +427,15 @@ repeat:
/* move the page to the destination cache */
spin_lock_irq(&smap->tree_lock);
page2 = radix_tree_delete(&smap->page_tree, offset);
- if (unlikely(page2 != page))
- NILFS_PAGE_BUG(page, "page removal failed "
- "(offset=%lu, page2=%p)",
- offset, page2);
+ WARN_ON(page2 != page);
+
smap->nrpages--;
spin_unlock_irq(&smap->tree_lock);
spin_lock_irq(&dmap->tree_lock);
err = radix_tree_insert(&dmap->page_tree, offset, page);
if (unlikely(err < 0)) {
- BUG_ON(err == -EEXIST);
+ WARN_ON(err == -EEXIST);
page->mapping = NULL;
page_cache_release(page); /* for cache */
} else {
diff --git a/fs/nilfs2/recovery.c b/fs/nilfs2/recovery.c
index a4253f3..ef387b1 100644
--- a/fs/nilfs2/recovery.c
+++ b/fs/nilfs2/recovery.c
@@ -92,9 +92,6 @@ static int nilfs_warn_segment_error(int err)
printk(KERN_WARNING
"NILFS warning: No super root in the last segment\n");
break;
- case NILFS_SEG_VALID:
- default:
- BUG();
}
return -EINVAL;
}
diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 24d0fbd..9a87410 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -334,8 +334,7 @@ static void nilfs_transaction_lock(struct nilfs_sb_info *sbi,
{
struct nilfs_transaction_info *cur_ti = current->journal_info;
- BUG_ON(cur_ti);
- BUG_ON(!ti);
+ WARN_ON(cur_ti);
ti->ti_flags = NILFS_TI_WRITER;
ti->ti_count = 0;
ti->ti_save = cur_ti;
@@ -546,8 +545,6 @@ static int nilfs_collect_file_data(struct nilfs_sc_info *sci,
{
int err;
- /* BUG_ON(!buffer_dirty(bh)); */
- /* excluded by scan_dirty_data_buffers() */
err = nilfs_bmap_propagate(NILFS_I(inode)->i_bmap, bh);
if (unlikely(err < 0))
return nilfs_handle_bmap_error(err, __func__, inode,
@@ -566,8 +563,6 @@ static int nilfs_collect_file_node(struct nilfs_sc_info *sci,
{
int err;
- /* BUG_ON(!buffer_dirty(bh)); */
- /* excluded by scan_dirty_node_buffers() */
err = nilfs_bmap_propagate(NILFS_I(inode)->i_bmap, bh);
if (unlikely(err < 0))
return nilfs_handle_bmap_error(err, __func__, inode,
@@ -579,7 +574,7 @@ static int nilfs_collect_file_bmap(struct nilfs_sc_info *sci,
struct buffer_head *bh,
struct inode *inode)
{
- BUG_ON(!buffer_dirty(bh));
+ WARN_ON(!buffer_dirty(bh));
return nilfs_segctor_add_file_block(sci, bh, inode, sizeof(__le64));
}
@@ -628,7 +623,7 @@ static int nilfs_collect_dat_data(struct nilfs_sc_info *sci,
static int nilfs_collect_dat_bmap(struct nilfs_sc_info *sci,
struct buffer_head *bh, struct inode *inode)
{
- BUG_ON(!buffer_dirty(bh));
+ WARN_ON(!buffer_dirty(bh));
return nilfs_segctor_add_file_block(sci, bh, inode,
sizeof(struct nilfs_binfo_dat));
}
@@ -862,9 +857,9 @@ static int nilfs_segctor_create_checkpoint(struct nilfs_sc_info *sci)
nilfs_mdt_mark_dirty(nilfs->ns_cpfile);
nilfs_cpfile_put_checkpoint(
nilfs->ns_cpfile, nilfs->ns_cno, bh_cp);
- } else {
- BUG_ON(err == -EINVAL || err == -ENOENT);
- }
+ } else
+ WARN_ON(err == -EINVAL || err == -ENOENT);
+
return err;
}
@@ -879,7 +874,7 @@ static int nilfs_segctor_fill_in_checkpoint(struct nilfs_sc_info *sci)
err = nilfs_cpfile_get_checkpoint(nilfs->ns_cpfile, nilfs->ns_cno, 0,
&raw_cp, &bh_cp);
if (unlikely(err)) {
- BUG_ON(err == -EINVAL || err == -ENOENT);
+ WARN_ON(err == -EINVAL || err == -ENOENT);
goto failed_ibh;
}
raw_cp->cp_snapshot_list.ssl_next = 0;
@@ -944,7 +939,6 @@ static void nilfs_fill_in_super_root_crc(struct buffer_head *bh_sr, u32 seed)
(struct nilfs_super_root *)bh_sr->b_data;
u32 crc;
- BUG_ON(NILFS_SR_BYTES > bh_sr->b_size);
crc = crc32_le(seed,
(unsigned char *)raw_sr + sizeof(raw_sr->sr_sum),
NILFS_SR_BYTES - sizeof(raw_sr->sr_sum));
@@ -1022,8 +1016,7 @@ static void nilfs_segctor_cancel_free_segments(struct nilfs_sc_info *sci,
if (!(ent->flags & NILFS_SLH_FREED))
break;
err = nilfs_sufile_cancel_free(sufile, ent->segnum);
- BUG_ON(err);
-
+ WARN_ON(err); /* do not happen */
ent->flags &= ~NILFS_SLH_FREED;
}
}
@@ -1472,7 +1465,7 @@ static int nilfs_segctor_extend_segments(struct nilfs_sc_info *sci,
failed:
list_for_each_entry_safe(segbuf, n, &list, sb_list) {
ret = nilfs_sufile_free(sufile, segbuf->sb_nextnum);
- BUG_ON(ret);
+ WARN_ON(ret); /* never fails */
list_del_init(&segbuf->sb_list);
nilfs_segbuf_free(segbuf);
}
@@ -1488,7 +1481,7 @@ static void nilfs_segctor_free_incomplete_segments(struct nilfs_sc_info *sci,
segbuf = NILFS_FIRST_SEGBUF(&sci->sc_segbufs);
if (nilfs->ns_nextnum != segbuf->sb_nextnum) {
ret = nilfs_sufile_free(nilfs->ns_sufile, segbuf->sb_nextnum);
- BUG_ON(ret);
+ WARN_ON(ret); /* never fails */
}
if (segbuf->sb_io_error) {
/* Case 1: The first segment failed */
@@ -1504,7 +1497,7 @@ static void nilfs_segctor_free_incomplete_segments(struct nilfs_sc_info *sci,
list_for_each_entry_continue(segbuf, &sci->sc_segbufs, sb_list) {
ret = nilfs_sufile_free(nilfs->ns_sufile, segbuf->sb_nextnum);
- BUG_ON(ret);
+ WARN_ON(ret); /* never fails */
if (!done && segbuf->sb_io_error) {
if (segbuf->sb_segnum != nilfs->ns_nextnum)
/* Case 2: extended segment (!= next) failed */
@@ -1558,7 +1551,7 @@ static void nilfs_segctor_update_segusage(struct nilfs_sc_info *sci,
list_for_each_entry(segbuf, &sci->sc_segbufs, sb_list) {
ret = nilfs_sufile_get_segment_usage(sufile, segbuf->sb_segnum,
&raw_su, &bh_su);
- BUG_ON(ret); /* always succeed because bh_su is dirty */
+ WARN_ON(ret); /* always succeed because bh_su is dirty */
live_blocks = segbuf->sb_sum.nblocks +
(segbuf->sb_pseg_start - segbuf->sb_fseg_start);
raw_su->su_lastmod = cpu_to_le64(sci->sc_seg_ctime);
@@ -1579,7 +1572,7 @@ static void nilfs_segctor_cancel_segusage(struct nilfs_sc_info *sci,
segbuf = NILFS_FIRST_SEGBUF(&sci->sc_segbufs);
ret = nilfs_sufile_get_segment_usage(sufile, segbuf->sb_segnum,
&raw_su, &bh_su);
- BUG_ON(ret); /* always succeed because bh_su is dirty */
+ WARN_ON(ret); /* always succeed because bh_su is dirty */
raw_su->su_nblocks = cpu_to_le32(segbuf->sb_pseg_start -
segbuf->sb_fseg_start);
nilfs_sufile_put_segment_usage(sufile, segbuf->sb_segnum, bh_su);
@@ -1587,7 +1580,7 @@ static void nilfs_segctor_cancel_segusage(struct nilfs_sc_info *sci,
list_for_each_entry_continue(segbuf, &sci->sc_segbufs, sb_list) {
ret = nilfs_sufile_get_segment_usage(sufile, segbuf->sb_segnum,
&raw_su, &bh_su);
- BUG_ON(ret); /* always succeed */
+ WARN_ON(ret); /* always succeed */
raw_su->su_nblocks = 0;
nilfs_sufile_put_segment_usage(sufile, segbuf->sb_segnum,
bh_su);
@@ -1606,7 +1599,7 @@ static void nilfs_segctor_truncate_segments(struct nilfs_sc_info *sci,
list_del_init(&segbuf->sb_list);
sci->sc_segbuf_nblocks -= segbuf->sb_rest_blocks;
ret = nilfs_sufile_free(sufile, segbuf->sb_nextnum);
- BUG_ON(ret);
+ WARN_ON(ret);
nilfs_segbuf_free(segbuf);
}
}
@@ -1923,7 +1916,6 @@ static int nilfs_page_has_uncleared_buffer(struct page *page)
static void __nilfs_end_page_io(struct page *page, int err)
{
- /* BUG_ON(err > 0); */
if (!err) {
if (!nilfs_page_buffers_clean(page))
__set_page_dirty_nobuffers(page);
@@ -2262,7 +2254,7 @@ static int nilfs_segctor_deactivate_segments(struct nilfs_sc_info *sci,
if (unlikely(err))
goto failed;
nilfs_segment_usage_clear_active(ent->raw_su);
- BUG_ON(!buffer_dirty(ent->bh_su));
+ WARN_ON(!buffer_dirty(ent->bh_su));
}
return 0;
@@ -2340,7 +2332,6 @@ static int nilfs_segctor_do_construct(struct nilfs_sc_info *sci, int mode)
/* Avoid empty segment */
if (sci->sc_stage.scnt == NILFS_ST_DONE &&
NILFS_SEG_EMPTY(&sci->sc_curseg->sb_sum)) {
- BUG_ON(mode == SC_LSEG_SR);
nilfs_segctor_end_construction(sci, nilfs, 1);
goto out;
}
@@ -2479,9 +2470,8 @@ int nilfs_segctor_add_segments_to_be_freed(struct nilfs_sc_info *sci,
struct inode *sufile = nilfs->ns_sufile;
LIST_HEAD(list);
__u64 *pnum;
- const char *flag_name;
size_t i;
- int err, err2 = 0;
+ int err;
for (pnum = segnum, i = 0; i < nsegs; pnum++, i++) {
ent = nilfs_alloc_segment_entry(*pnum);
@@ -2495,32 +2485,12 @@ int nilfs_segctor_add_segments_to_be_freed(struct nilfs_sc_info *sci,
if (unlikely(err))
goto failed;
- if (unlikely(le32_to_cpu(ent->raw_su->su_flags) !=
- (1UL << NILFS_SEGMENT_USAGE_DIRTY))) {
- if (nilfs_segment_usage_clean(ent->raw_su))
- flag_name = "clean";
- else if (nilfs_segment_usage_active(ent->raw_su))
- flag_name = "active";
- else if (nilfs_segment_usage_volatile_active(
- ent->raw_su))
- flag_name = "volatile active";
- else if (!nilfs_segment_usage_dirty(ent->raw_su))
- flag_name = "non-dirty";
- else
- flag_name = "erroneous";
-
- printk(KERN_ERR
- "NILFS: %s segment is requested to be cleaned "
- "(segnum=%llu)\n",
- flag_name, (unsigned long long)ent->segnum);
- err2 = -EINVAL;
- }
+ if (unlikely(!nilfs_segment_usage_dirty(ent->raw_su)))
+ printk(KERN_WARNING "NILFS: unused segment is "
+ "requested to be cleaned (segnum=%llu)\n",
+ (unsigned long long)ent->segnum);
nilfs_close_segment_entry(ent, sufile);
}
- if (unlikely(err2)) {
- err = err2;
- goto failed;
- }
list_splice(&list, sci->sc_cleaning_segments.prev);
return 0;
@@ -2705,8 +2675,6 @@ struct nilfs_segctor_req {
static void nilfs_segctor_accept(struct nilfs_sc_info *sci,
struct nilfs_segctor_req *req)
{
- BUG_ON(!sci);
-
req->sc_err = req->sb_err = 0;
spin_lock(&sci->sc_state_lock);
req->seq_accepted = sci->sc_seq_request;
@@ -3107,7 +3075,7 @@ static void nilfs_segctor_destroy(struct nilfs_sc_info *sci)
if (flag || nilfs_segctor_confirm(sci))
nilfs_segctor_write_out(sci);
- BUG_ON(!list_empty(&sci->sc_copied_buffers));
+ WARN_ON(!list_empty(&sci->sc_copied_buffers));
if (!list_empty(&sci->sc_dirty_files)) {
nilfs_warning(sbi->s_super, __func__,
@@ -3120,7 +3088,7 @@ static void nilfs_segctor_destroy(struct nilfs_sc_info *sci)
if (!list_empty(&sci->sc_cleaning_segments))
nilfs_dispose_segment_list(&sci->sc_cleaning_segments);
- BUG_ON(!list_empty(&sci->sc_segbufs));
+ WARN_ON(!list_empty(&sci->sc_segbufs));
if (sci->sc_sketch_inode) {
iput(sci->sc_sketch_inode);
diff --git a/fs/nilfs2/sufile.c b/fs/nilfs2/sufile.c
index cc714c7..4cf47e0 100644
--- a/fs/nilfs2/sufile.c
+++ b/fs/nilfs2/sufile.c
@@ -231,10 +231,11 @@ int nilfs_sufile_cancel_free(struct inode *sufile, __u64 segnum)
kaddr = kmap_atomic(su_bh->b_page, KM_USER0);
su = nilfs_sufile_block_get_segment_usage(
sufile, segnum, su_bh, kaddr);
- if (!nilfs_segment_usage_clean(su)) {
- printk(KERN_CRIT "%s: segment %llu must be clean\n",
+ if (unlikely(!nilfs_segment_usage_clean(su))) {
+ printk(KERN_WARNING "%s: segment %llu must be clean\n",
__func__, (unsigned long long)segnum);
- BUG();
+ kunmap_atomic(kaddr, KM_USER0);
+ goto out_su_bh;
}
nilfs_segment_usage_set_dirty(su);
kunmap_atomic(kaddr, KM_USER0);
@@ -249,11 +250,10 @@ int nilfs_sufile_cancel_free(struct inode *sufile, __u64 segnum)
nilfs_mdt_mark_buffer_dirty(su_bh);
nilfs_mdt_mark_dirty(sufile);
+ out_su_bh:
brelse(su_bh);
-
out_header:
brelse(header_bh);
-
out_sem:
up_write(&NILFS_MDT(sufile)->mi_sem);
return ret;
@@ -317,7 +317,7 @@ int nilfs_sufile_freev(struct inode *sufile, __u64 *segnum, size_t nsegs)
kaddr = kmap_atomic(su_bh[i]->b_page, KM_USER0);
su = nilfs_sufile_block_get_segment_usage(
sufile, segnum[i], su_bh[i], kaddr);
- BUG_ON(nilfs_segment_usage_error(su));
+ WARN_ON(nilfs_segment_usage_error(su));
nilfs_segment_usage_set_clean(su);
kunmap_atomic(kaddr, KM_USER0);
nilfs_mdt_mark_buffer_dirty(su_bh[i]);
@@ -385,8 +385,8 @@ int nilfs_sufile_get_segment_usage(struct inode *sufile, __u64 segnum,
int ret;
/* segnum is 0 origin */
- BUG_ON(segnum >= nilfs_sufile_get_nsegments(sufile));
-
+ if (segnum >= nilfs_sufile_get_nsegments(sufile))
+ return -EINVAL;
down_write(&NILFS_MDT(sufile)->mi_sem);
ret = nilfs_sufile_get_segment_usage_block(sufile, segnum, 1, &bh);
if (ret < 0)
@@ -515,6 +515,8 @@ int nilfs_sufile_get_ncleansegs(struct inode *sufile, unsigned long *nsegsp)
* %-EIO - I/O error.
*
* %-ENOMEM - Insufficient amount of memory available.
+ *
+ * %-EINVAL - Invalid segment usage number.
*/
int nilfs_sufile_set_error(struct inode *sufile, __u64 segnum)
{
@@ -524,8 +526,11 @@ int nilfs_sufile_set_error(struct inode *sufile, __u64 segnum)
void *kaddr;
int ret;
- BUG_ON(segnum >= nilfs_sufile_get_nsegments(sufile));
-
+ if (unlikely(segnum >= nilfs_sufile_get_nsegments(sufile))) {
+ printk(KERN_WARNING "%s: invalid segment number: %llu\n",
+ __func__, (unsigned long long)segnum);
+ return -EINVAL;
+ }
down_write(&NILFS_MDT(sufile)->mi_sem);
ret = nilfs_sufile_get_header_block(sufile, &header_bh);
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 2f0e9f7..d0639a6 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -841,8 +841,11 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent,
if (sb->s_flags & MS_RDONLY) {
if (nilfs_test_opt(sbi, SNAPSHOT)) {
- if (!nilfs_cpfile_is_snapshot(nilfs->ns_cpfile,
- sbi->s_snapshot_cno)) {
+ err = nilfs_cpfile_is_snapshot(nilfs->ns_cpfile,
+ sbi->s_snapshot_cno);
+ if (err < 0)
+ goto failed_sbi;
+ if (!err) {
printk(KERN_ERR
"NILFS: The specified checkpoint is "
"not a snapshot "
@@ -1163,7 +1166,6 @@ nilfs_get_sb(struct file_system_type *fs_type, int flags,
} else {
struct nilfs_sb_info *sbi = NILFS_SB(s);
- BUG_ON(!sbi || !sbi->s_nilfs);
/*
* s_umount protects super_block from unmount process;
* It covers pointers of nilfs_sb_info and the_nilfs.
--
1.5.6.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH mmotm 6/6] nilfs2: replace BUG_ON and BUG calls triggerable from ioctl
2009-02-17 7:42 ` [PATCH mmotm 6/6] nilfs2: replace BUG_ON and BUG calls triggerable from ioctl Ryusuke Konishi
@ 2009-02-17 11:28 ` Pekka Enberg
0 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2009-02-17 11:28 UTC (permalink / raw)
To: Ryusuke Konishi; +Cc: Andrew Morton, linux-fsdevel, linux-kernel
On Tue, 2009-02-17 at 16:42 +0900, Ryusuke Konishi wrote:
> Pekka Enberg advised me:
> > It would be nice if BUG(), BUG_ON(), and panic() calls would be
> > converted to proper error handling using WARN_ON() calls. The BUG()
> > call in nilfs_cpfile_delete_checkpoints(), for example, looks to be
> > triggerable from user-space via the ioctl() system call.
>
> This will follow the comment and keep them to a minimum.
>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
Looks good to me!
Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>
^ permalink raw reply [flat|nested] 11+ messages in thread