On Wednesday, July 24, 2013 08:06:27 AM Greg KH wrote: > On Wed, Jul 24, 2013 at 01:54:56PM +0200, Bartlomiej Zolnierkiewicz wrote: > > > > On Wednesday, July 24, 2013 01:41:57 PM Michal Hocko wrote: > > > On Wed 24-07-13 12:32:32, Bartlomiej Zolnierkiewicz wrote: > > > > > > > > Hi, > > > > > > > > On Wednesday, July 24, 2013 12:04:41 PM Michal Hocko wrote: > > > > > On Wed 24-07-13 12:02:35, Michal Hocko wrote: > > > > > > I have posted a similar fix quite some time ago and I guess Greg should > > > > > > already have it. > > > > > > > > > > For reference https://lkml.org/lkml/2013/6/26/410 > > > > > > > > There was a reply from Greg: > > > > > > > > https://lkml.org/lkml/2013/6/26/410 > > > > > > I guess you meant https://lkml.org/lkml/2013/6/26/569 > > > > Uh, yes. > > > > > > and it seems that Greg's request has not been fullfilled. > > > > > > Bob Liu has resent the patch (I cannot find the email in the archive). > > > > Indeed, I've found it here: > > > > http://comments.gmane.org/gmane.linux.kernel.mm/102484 > > > > > > Anyway Piotr's patch seems to be more complete: > > > > - it also fixes case in which invalid "zcache=" option is given (returns > > > > an error value 1 while with your patch the code still erronously retuns 0) > > > > - it prints information about compressor type being used when "zcache=" > > > > option is used (your patch skips it due to addition of "goto out_alloc") > > > > > > I do not care which one will make it I just pointed out that there is > > > another patch dealing with the same and it is a question how far that > > > one went. > > > > OK. > > > > Greg, could you please pick Piotr's patch? > > Wait, I'm going to take the first one I got, from Michal, as that's the > original author here, not Piotr... > > confused, and really grumpy about the whole zcache mess... Just get the second one (from Piotr) as it is more complete (it fixes also problem with invalid data being given to "zcache=" option or chosen compressor not being supported by Crypto API). I feel a bit sad that this newer patch makes Michal's one obsolete but I don't have a good idea how to resolve it in other way while sticking to "chose the best technical solution" principle. :( Oh, wait. I updated patch (attached below) to give Michal & Cristian some credit (also added Konrad's Acked-by while at it). I hope that this is an acceptable solution. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics From: Piotr Sarna <p.sarna@xxxxxxxxxxxxxxxxxxx> Subject: [PATCH] zcache: fix "zcache=" kernel parameter Commit 835f2f5 ("staging: zcache: enable zcache to be built/loaded as a module") introduced an incorrect handling of "zcache=" parameter. Inside zcache_comp_init() function, zcache_comp_name variable is checked for being empty. If not empty, the above variable is tested for being compatible with Crypto API. Unfortunately, after that function ends unconditionally (by the "goto out" directive) and returns: - non-zero value if verification succeeded, wrongly indicating an error - zero value if verification failed, falsely informing that function zcache_comp_init() ended properly. A solution to this problem is as following: 1. Move the "goto out" directive inside the "if (!ret)" statement 2. In case that crypto_has_comp() returned 0, change the value of ret to non-zero before "goto out" to indicate an error. This patch replaces an earlier one from Michal Hocko (based on report from Cristian Rodríguez): http://comments.gmane.org/gmane.linux.kernel.mm/102484 It also addressed the same issue but didn't fix the zcache_comp_init() for case when the compressor data passed to "zcache=" option was invalid or unsupported. Signed-off-by: Piotr Sarna <p.sarna@xxxxxxxxxxxxxxxxxxx> [bzolnier: updated patch description] Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Cc: Cristian Rodríguez <crrodriguez@xxxxxxxxxxxx> Cc: Michal Hocko <mhocko@xxxxxxx> Cc: Bob Liu <bob.liu <at> oracle.com> --- drivers/staging/zcache/zcache-main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c index dcceed2..81972fa 100644 --- a/drivers/staging/zcache/zcache-main.c +++ b/drivers/staging/zcache/zcache-main.c @@ -1811,10 +1811,12 @@ static int zcache_comp_init(void) #else if (*zcache_comp_name != '\0') { ret = crypto_has_comp(zcache_comp_name, 0, 0); - if (!ret) + if (!ret) { pr_info("zcache: %s not supported\n", zcache_comp_name); - goto out; + ret = 1; + goto out; + } } if (!ret) strcpy(zcache_comp_name, "lzo"); -- 1.7.9.5 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
