On Mon, 31 Aug 2009 16:37:29 +0900 (JST), Ryusuke Konishi wrote:
> On Sat, 29 Aug 2009 17:54:30 +0900, Jiro SEKIBA <[email protected]> wrote:
> > nilfs = NILFS_SB(inode->i_sb)->s_nilfs;
> >
> > + if (nilfs_gc_running(nilfs))
> > + return -EBUSY;
> > + set_nilfs_gc_running(nilfs);
> > +
>
> Well, I have two comments here on this patch:
>
> This check and the error return should be done before memory
> allocation of kbufs[4] to avoid memory leak.
>
> And, logically these bit operations must be atomic by use of
> test_and_set_bit(). So, it should be like:
>
> if (test_and_set_bit(THE_NILFS_GC_RUNNING, &nilfs->ns_flags))
> return -EBUSY;
>
> I think we don't have to expand THE_NILFS_FNS macro for this.
I mean we don't have to add the following template:
static inline void test_and_set_nilfs_##name(struct the_nilfs *nilfs) \
{ \
return test_and_set_bit(THE_NILFS_##bit, &(nilfs)->ns_flags); \
}
But, it's OK if you prefer this.
Just in case, the use of "THE_NILFS_FNS(GC_RUNNING, gc_running)" looks
proper; you don't have to change clear_nilfs_gc_running() to
"clear_bit(THE_NILFS_GC_RUNNING, &nilfs->ns_flags)".
Thanks,
Ryusuke Konishi
_______________________________________________
users mailing list
[email protected]
https://www.nilfs.org/mailman/listinfo/users
