Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751915AbcDZFHu (ORCPT ); Tue, 26 Apr 2016 01:07:50 -0400 Received: from ausc60ps301.us.dell.com ([143.166.148.206]:53758 "EHLO ausc60ps301.us.dell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751043AbcDZFHt (ORCPT ); Tue, 26 Apr 2016 01:07:49 -0400 X-Greylist: delayed 583 seconds by postgrey-1.27 at vger.kernel.org; Tue, 26 Apr 2016 01:07:48 EDT DomainKey-Signature: s=smtpout; d=dell.com; c=nofws; q=dns; h=X-LoopCount0:X-IronPort-AV:From:To:Cc:Subject:Date: Message-Id:X-Mailer; b=SWPDiyxakqWUIetwlqBr9A1UeiaYKjyFNuouvRBNwXHHuGQ6k6b/B52+ 6cuPHDz0EP4J8d4pKaHp+MfvrCzvqrCHz+6lIkTesH/9U7uniLxsnr0fZ pxuBQrcvdkp7FB1csACBUoaRoZVFn3y1JBpg+io5Zxl3IIiaYO/txRskv k=; X-LoopCount0: from 10.209.151.17 X-IronPort-AV: E=Sophos;i="5.24,535,1454997600"; d="scan'208";a="814558757" From: Mario Limonciello To: ming.lei@canonical.com Cc: LKML , Mario Limonciello , stable@vger.kernel.org, stuart_hayes@dell.com Subject: [PATCH 1/1] firmware: correct test of wait_for_completion_interruptible_timeout return Date: Mon, 25 Apr 2016 23:57:32 -0500 Message-Id: <1461646652-20393-1-git-send-email-mario_limonciello@dell.com> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2343 Lines: 57 Before this commit both code paths (hotplug and not-hotplug as determined by FW_OPT_UEVENT) would block on an interruptible completion, but the hotplugscenario also would wait until timeout and then abort. The non hotplugscenario (which dell-rbu followed) would block indefinitely until interrupted. After this commit both scenarios block on an interruptible condition but the hotplug scenario follows the value of firmware_loading_timeout to end. This changed the situation for the non hotplug scenario to instead wait for MAX_JIFFY_OFFSET. This should have the same result, but dell-rbu was failing with negative values returned from wait_for_completion_interruptible_timeout after completion occurred. This shouldn't be possible, but it was happening because the return is a long but the value it was being saved to was an int. When completion occurs quickly wait_for_completion_interruptible_timeout returns how much time was left (which happens to be a very big number since the timeout was MAX_JIFFY_OFFSET) One test run of mine returned 4611686018427368747. Other parts of the kernel with this type of return don't have problems because they use smaller timeouts. So to fix this: change the test to not store the value to an int, but rather directly compare against what wait_for_completion_interruptible_timeout can return. Fixes: 68ff2a00dbf59 (firmware_loader: handle timeout via wait_for_completion_interruptible_timeout()) CC: stable@vger.kernel.org CC: stuart_hayes@dell.com Signed-off-by: Mario Limonciello --- drivers/base/firmware_class.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 773fc30..223af70 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -917,14 +917,11 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, timeout = MAX_JIFFY_OFFSET; } - retval = wait_for_completion_interruptible_timeout(&buf->completion, - timeout); - if (retval == -ERESTARTSYS || !retval) { + if (wait_for_completion_interruptible_timeout(&buf->completion, + timeout) <= 0) { mutex_lock(&fw_lock); fw_load_abort(fw_priv); mutex_unlock(&fw_lock); - } else if (retval > 0) { - retval = 0; } if (is_fw_load_aborted(buf)) -- 2.7.4