From mboxrd@z Thu Jan 1 00:00:00 1970 From: Detlev Zundel Date: Thu, 30 Jun 2011 16:34:56 +0200 Subject: [U-Boot] [PATCH 2/2] ext2: Simplify partial sector access logic In-Reply-To: (Anton Staaf's message of "Wed, 29 Jun 2011 12:23:06 -0700") References: <1308001240-9545-1-git-send-email-robotboy@chromium.org> <1308001240-9545-3-git-send-email-robotboy@chromium.org> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Anton, > On Wed, Jun 29, 2011 at 6:04 AM, Detlev Zundel wrote: > >> Hi Anton, >> >> > Previously reading or writing zero full sectors (reading the end of >> > one sector and the beginning of the next for example) was special >> > cased and involved stack allocating a second sector buffer. This >> > change uses the same code path for this case as well as when there >> > are a non-zero number of full sectors to access. The result is >> > easier to read and reduces the maximum stack used. >> >> It's non-trivial to prove that your change is equivalent and >> unfortunately I do not have enough time to do this. If your tests work, >> than this is certainly a good indication ;) The one thing I'd like to be >> sure is that the previous code looks like it used the stack for whole >> sectors but copied only parts of this to the provided address pointer. >> Your new code looks like it always writes whole sectors to the provided >> pointer. Is this safe even if the caller allocated space without >> overhead for whole sectors? > > > Thanks for the reviews by the way. My new version of the code still bounces > partial sector reads (both at the beginning and end of the range) through a > stack allocated sector buffer. The portion that is writing directly to the > users buffer is only used for reading the full sectors. The middle section > (in the "if (sectors > 0)" block) is reading only as many sectors as are > specified by (byte_len / SECTOR_SIZE). byte_len, buf and sector at this > point in the function have been updated by the first block that deals with > reading the unaligned start of the data (if it exists). > > Also, I have tested this code on a Tegra board using ext2ls and ext2load of > a kernel image. Thanks for the explanation. The new code certainly reads cleaner so Acked-by: Detlev Zundel Thanks Detlev -- The management question ... is not _whether_ to build a pilot system and throw it away. You _will_ do that. The only question is whether to plan in advance to build a throwaway, or to promise to deliver the throwaway to customers. - Fred Brooks, "The Mythical Man Month" -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de