Re: [PATCH 3/3] nilfs2: stop using periodic write_super callback

[Ryusuke Konishi <ryusuke-sG5X7nlA6pw@public.gmane.org>]

On Sat, 18 Jul 2009 16:24:09 +0900, Jiro SEKIBA wrote:
> At Sat, 18 Jul 2009 03:04:21 +0900 (JST),
> Ryusuke Konishi wrote:
> > >  		if (test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) &&
> > >  		    nilfs_discontinued(nilfs)) {
> > 
> > Well, maybe we should change the name of nilfs_discontinued before
> > applying this patch.
> > 
> > It originally meant that the latest log got untraceable from
> > the log which the super block points to.
> > 
> > After applying this patch, the meaning of the flag will change to a
> > state "the superblock needs to be written back".
> 
> I haven't checked yet other place deeply, but if the meaning of the
> nilfs_dicontinued() is still used in the other place, how about 
> defining a macro like following:
> 
> #define nilfs_sb_needed_to_written_back(n) nilfs_discontined((n))

I don't think we have to define the intermediate step because the
related definitions are never used outside nilfs.

But, okay, let's leave renaming the `discontinued' until later. It's
not a priority.

> > > +		t = get_seconds();
> > > +		sbi = sci->sc_sbi;
> > > +	        nilfs = sbi->s_nilfs;
> > > +		if(test_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags) && 
> > > +	   	   nilfs_update_super(nilfs,t))
> > > +			set_nilfs_discontinued(nilfs);
> > 
> > We need to discuss for this.
> > 
> > At least, the NILFS_SC_SUPER_ROOT bit may be cleared by other tasks
> > outside the transaction lock.
> 
> I see, then it doesn't make sence to check this bit in the thread.
> Reason I've tested the bit is that nilfs_segctor_construct is checking
> that bit when commiting super block.
 
This bit means that the latest log has a super root block, so it's
cleared when the next log is written out.

It makes sense in nilfs_segctor_construct(), but it doesn't in the
thread function.

How about testing sci->sc_super->s_dirt instead ?

Regards,
Ryusuke Konishi