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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Mon, 19 Feb 2007 16:11:14 +0200
From:	Artem Bityutskiy <dedekind@...radead.org>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.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

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 (Битюцкий Артём)

-
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/