From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967516AbeE2VVE (ORCPT ); Tue, 29 May 2018 17:21:04 -0400 Received: from mail-qk0-f193.google.com ([209.85.220.193]:45802 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S966946AbeE2VU7 (ORCPT ); Tue, 29 May 2018 17:20:59 -0400 X-Google-Smtp-Source: ADUXVKIye5USyjX8GB/e7J3t1EEGKURZxSg2F14zcWzbILFLy4g8SN7vVn9D84y+AclPyygiBIBv6DPEG+X3sYf6p6o= MIME-Version: 1.0 In-Reply-To: References: From: Andy Shevchenko Date: Wed, 30 May 2018 00:20:57 +0300 Message-ID: Subject: Re: [PATCH] PCI: hv: Do not wait forever on a device that has disappeared To: Dexuan Cui Cc: Lorenzo Pieralisi , Bjorn Helgaas , "linux-pci@vger.kernel.org" , KY Srinivasan , Stephen Hemminger , "olaf@aepfle.de" , "apw@canonical.com" , "jasowang@redhat.com" , "linux-kernel@vger.kernel.org" , "driverdev-devel@linuxdriverproject.org" , Haiyang Zhang , "vkuznets@redhat.com" , "marcelo.cerri@canonical.com" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 24, 2018 at 12:12 AM, Dexuan Cui wrote: > > Before the guest finishes the device initialization, the device can be > removed anytime by the host, and after that the host won't respond to > the guest's request, so the guest should be prepared to handle this > case. > + while (true) { > + if (hdev->channel->rescind) { > + dev_warn_once(&hdev->device, "The device is gone.\n"); > + return -ENODEV; > + } > + > + if (wait_for_completion_timeout(comp, HZ / 10)) > + break; > + } Infinite loops are usually a red flags. What's wrong with simple: do { ... } while (wait_...(...) == 0); ? > + if (!ret) > + ret = wait_for_response(hdev, &comp); Better to use well established patterns, i.e. if (ret) return ret; > + if (!ret) > + ret = wait_for_response(hdev, &comp_pkt.host_event); Here it looks okay on the first glance, but better to think about it again and refactor. -- With Best Regards, Andy Shevchenko From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id B906F1C0DE1 for ; Tue, 29 May 2018 21:20:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id B619085E4A for ; Tue, 29 May 2018 21:20:59 +0000 (UTC) Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id g1sXUPqMbq58 for ; Tue, 29 May 2018 21:20:59 +0000 (UTC) Received: from mail-qk0-f196.google.com (mail-qk0-f196.google.com [209.85.220.196]) by fraxinus.osuosl.org (Postfix) with ESMTPS id 1F68684D59 for ; Tue, 29 May 2018 21:20:59 +0000 (UTC) Received: by mail-qk0-f196.google.com with SMTP id j12-v6so9380432qkk.4 for ; Tue, 29 May 2018 14:20:59 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: Andy Shevchenko Date: Wed, 30 May 2018 00:20:57 +0300 Message-ID: Subject: Re: [PATCH] PCI: hv: Do not wait forever on a device that has disappeared List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Dexuan Cui Cc: "olaf@aepfle.de" , Lorenzo Pieralisi , Stephen Hemminger , "linux-pci@vger.kernel.org" , "jasowang@redhat.com" , "driverdev-devel@linuxdriverproject.org" , "linux-kernel@vger.kernel.org" , "apw@canonical.com" , "marcelo.cerri@canonical.com" , Bjorn Helgaas , "vkuznets@redhat.com" , Haiyang Zhang On Thu, May 24, 2018 at 12:12 AM, Dexuan Cui wrote: > > Before the guest finishes the device initialization, the device can be > removed anytime by the host, and after that the host won't respond to > the guest's request, so the guest should be prepared to handle this > case. > + while (true) { > + if (hdev->channel->rescind) { > + dev_warn_once(&hdev->device, "The device is gone.\n"); > + return -ENODEV; > + } > + > + if (wait_for_completion_timeout(comp, HZ / 10)) > + break; > + } Infinite loops are usually a red flags. What's wrong with simple: do { ... } while (wait_...(...) == 0); ? > + if (!ret) > + ret = wait_for_response(hdev, &comp); Better to use well established patterns, i.e. if (ret) return ret; > + if (!ret) > + ret = wait_for_response(hdev, &comp_pkt.host_event); Here it looks okay on the first glance, but better to think about it again and refactor. -- With Best Regards, Andy Shevchenko _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel