On Thu, Jan 17, 2019 at 11:29:06AM +0100, Christophe Leroy wrote: [...] > > > /* MEM2 64MB@0x10000000 */ > > > delta = wii_hole_start + wii_hole_size; > > > + if (__map_without_bats) > > > + return delta; > > > + > > > > Nothing is visibly broken without this patch, even with > > CONFIG_DEBUG_PAGEALLOC (tested on top of v5.0-rc2), but the patch still > > looks correct. > > Obviously, CONFIG_DEBUG_PAGEALLOC cannot work without this patch. > The purpose of CONFIG_DEBUG_PAGEALLOC is to unmap unused parts of memory so > that any access to them will pagefault. > > As this function inconditionnaly sets a BAT for the second block of RAM, any > access to free area in the upper block will be granted without a fault. > > I think you can test it by doing a kmalloc() then a kfree(), then try to > read in that area (hopefully kmalloc() allocates memory from the top so it > should go in the upper block). Maybe there is an LKDTM test for that. Ah, makes sense, thanks for the explanation. > > > > > I'd prefer the 'if' block to be before the whole delta/size calculation, > > to make the code slightly more readable because the delta and size > > calculations stay in one visual block. It doesn't need to happen after > > delta is calculated. > > Euh ... the function has to return 'delta', so if I put the if block before > the calculation of delta, it means we have to calculate delta twice: Oh, sorry, I misread the code, but you're completely right (I shouldn't answer mails while tired). > > if (__map_without_bats) > return wii_hole_start + wii_hole_size; > > delta = wii_hole_start + wii_hole_size; > > My eyes don't really like it, so if we want to keep delta and size > calculation together, the 'if' will go after calculation of size. I agree. > In anycase, this change is only really for fixing stable releases because > this function will go away with my other serie. ACK Thanks, Jonathan
Attachment:
signature.asc
Description: PGP signature
