Hi,
At Wed, 02 Sep 2009 22:20:35 +0900 (JST),
Ryusuke Konishi wrote:
>
> 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.
That's right. sorry for the careless misses.
I'll update the patch.
thank you for the review and suggestions
> 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
>
>
>
--
Jiro SEKIBA <[email protected]>
_______________________________________________
users mailing list
[email protected]
https://www.nilfs.org/mailman/listinfo/users