On Sat, Aug 6, 2011 at 2:13 AM, Dan Magenheimer <dan.magenheimer@xxxxxxxxxx> wrote: >> From: Bob Liu [mailto:lliubbo@xxxxxxxxx] >> Subject: Re: [PATCH 2/4] frontswap: using vzalloc instead of vmalloc >> >> On Fri, Aug 5, 2011 at 10:45 AM, Dan Magenheimer >> <dan.magenheimer@xxxxxxxxxx> wrote: >> >> > I am fairly sure that the failed allocation is handled gracefully >> >> > through the remainder of the frontswap code, but will re-audit to >> >> > confirm. A warning might be nice though. >> >> >> >> There is a place i think maybe have problem. >> >> function __frontswap_flush_area() in file frontswap.c called >> >> memset(sis->frontswap_map, .., ..); >> >> But if frontswap_map allocation fail there is a null pointer access ? >> > >> > Good catch! >> > >> > I'll fix that when I submit a frontswap update in a few days. >> >> Would you please add current patch to you frontswap update series ? >> So I needn't to send a Version 2 separately with only drop the >> allocation failed handler. >> Thanks. >> Regards, >> --Bob > > Hi Bob -- > > I'm not an expert here, so you or others can feel free to correct me if I've > got this wrong or if I misunderstood you, but I don't think that's the way > patchsets are supposed to be done, at least until they are merged into Linus' > tree. I think you are asking me to add a fifth patch in the frontswap > patch series that fixes this bug, rather than incorporate the fix into > the next posted version of the frontswap patchset. However, I expect > to post V5 soon with some additional (minor syntactic) changes to the > patchset from Konrad Wilk's very thorough review. Then this V5 will > replace the current version in linux-next soon thereafter (and hopefully > then into linux-3.2.) So I think it would be the correct process for me > to include your bugfix (with an acknowledgement in the commit log) in > that posted V5. > Yes, but current patch "frontswap: using vzalloc instead of vmalloc" has the error handler which is unneeded. + if (!frontswap_map) + goto bad_swap; If you want to include it into your series you must delete it by yourself(or I send an new one) and then add an extra patch which fix the frontswap_map null pointer bug into your series too. That's what I want. Sorry for the noise :). > That said, if you are using frontswap V4 (the version currently in > linux-next), the bug fix we've discussed needs to be fixed but is > exceedingly unlikely to occur in the real world because it would > require the malloc of swap_map to succeed (which is 8 bits per swap page > in the swapon'ed swap device) but the malloc of frontswap_map immediately > thereafter to fail (which is 1 bit per swap page in the swapon'ed swap > device). (And also this is not a problem for the vast majority of > kernel developers... it's only possible for frontswap users like you that > have enabled zcache or tmem or RAMster via a kernel boot option.) > > Thanks, > Dan > -- Regards, --Bob -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href