Hi Sekiba-san,
On Wed, 2 Sep 2009 15:29:50 +0900, Jiro SEKIBA <[email protected]> wrote:
> Hi,
>
> This is a revised patch to shorten freeze period.
> This version includes fixes of the bugs Konishi-san mentioned.
> <snip>
> @@ -509,6 +503,12 @@ static int nilfs_ioctl_clean_segments(struct inode
> *inode, struct file *filp,
> nsegs = argv[4].v_nmembs;
> if (argv[4].v_size != argsz[4])
> return -EINVAL;
> +
> + nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
> +
> + if (test_and_set_bit(THE_NILFS_GC_RUNNING, &nilfs->ns_flags))
> + return -EBUSY;
> +
> /*
> * argv[4] points to segment numbers this ioctl cleans. We
> * use kmalloc() for its buffer because memory used for the
> @@ -519,8 +519,6 @@ static int nilfs_ioctl_clean_segments(struct inode
> *inode, struct file *filp,
> if (IS_ERR(kbufs[4]))
> return PTR_ERR(kbufs[4]);
Hmmm, moving forward the test-and-set operation was not good idea.
Here is another problem. When memory allocation fails, the process
will return this function without clearing the GC bit.
How about putting bit operations adjacent to the GC routines as
follows ?
if (test_and_set_bit(THE_NILFS_GC_RUNNING, &nilfs->ns_flags)) {
ret = -EBUSY;
goto out_free;
}
ret = nilfs_ioctl_move_blocks(nilfs, &argv[0], kbufs[0]);
if (ret < 0)
printk(KERN_ERR "NILFS: GC failed during preparation: "
"cannot read source blocks: err=%d\n", ret);
else
ret = nilfs_clean_segments(inode->i_sb, argv, kbufs);
clear_nilfs_gc_running(nilfs);
I think it's also preferable to highlight what we should protect.
Thanks,
Ryusuke Konishi.
_______________________________________________
users mailing list
[email protected]
https://www.nilfs.org/mailman/listinfo/users
