Re: [PATCH 2/4] frontswap: using vzalloc instead of vmalloc

[Bob Liu <lliubbo@gmail.com>]

On Sat, Aug 6, 2011 at 2:13 AM, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
>> From: Bob Liu [mailto:lliubbo@gmail.com]
>> Subject: Re: [PATCH 2/4] frontswap: using vzalloc instead of vmalloc
>>
>> On Fri, Aug 5, 2011 at 10:45 AM, Dan Magenheimer
>> <dan.magenheimer@oracle.com> 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@kvack.org.  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=mailto:"dont@kvack.org"> email@kvack.org </a>