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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070219110543.GE16930@infradead.org>
Date:	Mon, 19 Feb 2007 11:05:43 +0000
From:	Christoph Hellwig <hch@...radead.org>
To:	Artem Bityutskiy <dedekind@...radead.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Christoph Hellwig <hch@...radead.org>,
	Frank Haverkamp <haver@...t.ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	David Woodhouse <dwmw2@...radead.org>,
	Josh Boyer <jwboyer@...ux.vnet.ibm.com>
Subject: Re: [PATCH 16/44 take 2] [UBI] scanning unit implementation

> +	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?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/