> From: Dan Carpenter [mailto:error27@xxxxxxxxx] > > On Thu, Jun 30, 2011 at 04:28:14PM -0700, Dan Magenheimer wrote: > > Hi Dan -- > > > > Thanks for the careful review. You're right... some > > of this was leftover from debugging an off-by-one error, > > though the code as is still works. > > > > OTOH, there's a good chance that much of this sysfs > > code will disappear before zcache would get promoted > > out of staging, since it is to help those experimenting > > with zcache to get more insight into what the underlying > > compression/accept-reject algorithms are doing. > > > > So I hope you (and GregKH) are OK that another version is > > not necessary at this time to fix these. > > Off by one errors are kind of insidious. People cut and paste them > and they spread. If someone adds a new list of chunks then there > are now two examples that are correct and two which have an extra > element, so it's 50/50 that he'll copy the right one. True, but these are NOT off-by-one errors... they are correct-but-slightly-ugly code snippets. (To clarify, I said the *ugliness* arose when debugging an off-by-one error.) Patches always welcome, and I agree that these should be fixed eventually, assuming the code doesn't go away completely first.. I'm simply stating the position that going through another test/submit cycling to fix correct-but-slightly-ugly code which is present only to surface information for experiments is not high on my priority list right now... unless GregKH says he won't accept the patch. > Btw, looking at it again, this seems like maybe a similar issue in > zbud_evict_zbpg(): > > 516 for (i = 0; i < MAX_CHUNK; i++) { > 517 retry_unbud_list_i: > > > MAX_CHUNKS is NCHUNKS - 1. Shouldn't that be i < NCHUNKS so that we > reach the last element in the list? No, the last element in that list is unused. There is a comment to that effect someplace in the code. (These lists are keeping track of pages with "chunks" of available space and the last entry would have no available space so is always empty.) Thanks again for your interest... are you using zcache? Dan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
