Re: [PATCH 16/44 take 2] [UBI] scanning unit implementation

[Artem Bityutskiy <dedekind@infradead.org>]

On Mon, 2007-02-19 at 11:05 +0000, Christoph Hellwig wrote:
> > +	cond_resched();
> > +	list_for_each_entry(seb, &si->erase, u.list)
> > +		if (seb->ec == NAND_SCAN_UNKNOWN_EC)
> > +			seb->ec = si->mean_ec;
> 
> You really shouldn't need random cond_resched all over the place.

Good point. I will review the code and clean up stuff like this.

While we are on the point, could you please formulate your criteria of
when cond_resched() should be used.

> > +static int compare_lebs(const struct ubi_info *ubi,
> > +			const struct ubi_scan_leb *seb, int pnum,
> > +			const struct ubi_vid_hdr *vid_hdr);
> 
> forward declarations in the middle of the file are really annoying.  Please try
> to reorder the code to not need them at all if needed, and if needed add them
> to the top of the file.

I tried to layout the code like: top level functions first, low level
last. This is matter of taste, isn't it? I am reluctant to change this,
because it is big useless work (I'll need to change it everywhere in
UBI). But will do this with great delight if it annoys too many people
(you are the second so far).

> > +	list_for_each_entry_safe(seb, seb_tmp, &si->alien, u.list) {
> > +		list_del(&seb->u.list);
> > +		ubi_free_scan_leb(seb);
> > +	}
> 
> no locking needed here?

No, initialization stage does not need any locking.


Thanks,
Artem.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)