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

Christoph Hellwig
2007-02-19T11:05:45Z

> + for (pnum = 0; pnum < io->peb_count; pnum++) {
> + cond_resched();
> +
> + err = process_eb(ubi, si, pnum);
> + if (unlikely(err < 0))
> + goto out_vidh;
> + }

> + rb_for_each_entry(rb1, sv, &si->volumes, rb) {
> + cond_resched();
> + rb_for_each_entry(rb2, seb, &sv->root, u.rb)
> + if (seb->ec == NAND_SCAN_UNKNOWN_EC)
> + seb->ec = si->mean_ec;
> + }
> +
> + cond_resched();
> + list_for_each_entry(seb, &si->free, u.list)
> + if (seb->ec == NAND_SCAN_UNKNOWN_EC)
> + seb->ec = si->mean_ec;
> +
> + cond_resched();
> + list_for_each_entry(seb, &si->corr, u.list)
> + if (seb->ec == NAND_SCAN_UNKNOWN_EC)
> + seb->ec = si->mean_ec;
> +
> + 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.

> +static int vid_hdr_sanity_check(const struct ubi_info *ubi,
> + const struct ubi_vid_hdr *vid_hdr,
> + const struct ubi_scan_volume *sv, int pnum);
> +
> +static int add_to_erase(struct ubi_scan_info *si, int pnum, int ec);
> +
> +static struct ubi_scan_volume *add_volume(const struct ubi_info *ubi,
> + struct ubi_scan_info *si, int vol_id,
> + int pnum,
> + const struct ubi_vid_hdr *vid_hdr);
> +
> +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.

> +void ubi_scan_destroy_si(struct ubi_scan_info *si)
> +{
> + struct ubi_scan_leb *seb, *seb_tmp;
> + struct ubi_scan_volume *sv;
> + struct rb_node *rb;
> +
> + 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?