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

Reply via email to