From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 004CDC10F0E for ; Mon, 15 Apr 2019 14:18:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C9D8320825 for ; Mon, 15 Apr 2019 14:18:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727354AbfDOOSu (ORCPT ); Mon, 15 Apr 2019 10:18:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47970 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725789AbfDOOSt (ORCPT ); Mon, 15 Apr 2019 10:18:49 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 794FB81F0E; Mon, 15 Apr 2019 14:18:49 +0000 (UTC) Received: from gondolin (unknown [10.40.205.58]) by smtp.corp.redhat.com (Postfix) with ESMTP id 83F455C1B4; Mon, 15 Apr 2019 14:18:45 +0000 (UTC) Date: Mon, 15 Apr 2019 16:18:41 +0200 From: Cornelia Huck To: Farhan Ali Cc: kvm@vger.kernel.org, linux-s390@vger.kernel.org, farman@linux.ibm.com, pasic@linux.ibm.com, pmorel@linux.ibm.com Subject: Re: [RFC v2 2/3] vfio-ccw: Prevent quiesce function going into an infinite loop Message-ID: <20190415161841.75aa61f4.cohuck@redhat.com> In-Reply-To: <00c54029-9f60-4242-8a3a-a87bf1e0434b@linux.ibm.com> References: <2c17cf29fbce8fc1cfbf60cfd04559d00c8eeac0.1554756534.git.alifm@linux.ibm.com> <20190411182434.07d5f685.cohuck@redhat.com> <20190412101013.2bf4a5df.cohuck@redhat.com> <396cde69-5c1d-b9e5-aaa2-248cf91e6f60@linux.ibm.com> <20190415101332.7ebbe5ad.cohuck@redhat.com> <00c54029-9f60-4242-8a3a-a87bf1e0434b@linux.ibm.com> Organization: Red Hat GmbH MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 15 Apr 2019 14:18:49 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Mon, 15 Apr 2019 09:38:37 -0400 Farhan Ali wrote: > On 04/15/2019 04:13 AM, Cornelia Huck wrote: > > On Fri, 12 Apr 2019 10:38:50 -0400 > > Farhan Ali wrote: > > > >> On 04/12/2019 04:10 AM, Cornelia Huck wrote: > >>> On Thu, 11 Apr 2019 16:30:44 -0400 > >>> Farhan Ali wrote: > >>> > >>>> On 04/11/2019 12:24 PM, Cornelia Huck wrote: > >>>>> On Mon, 8 Apr 2019 17:05:32 -0400 > >>>>> Farhan Ali wrote: > > > >>>>> Looking at the possible return codes: > >>>>> * -ENODEV -> device is not operational anyway, in theory you should even > >>>>> not need to bother with disabling the subchannel > >>>>> * -EIO -> we've run out of retries, and the subchannel still is not > >>>>> idle; I'm not sure if we could do anything here, as disable is > >>>>> unlikely to work, either > > > > (...) > > > >> Thinking a little bit more about EIO, if the return code is EIO then it > >> means we have exhausted all our options with cancel_halt_clear and the > >> subchannel/device is still status pending, right? > > > > Yes. > > > >> > >> I think we should still continue to try and disable the subchannel, > >> because if not then the subchannel/device could in some point of time > >> come back and bite us. So we really should protect the system from this > >> behavior. > > > > I think trying to disable the subchannel does not really hurt, but I > > fear it won't succeed in that case... > > > >> > >> I think for EIO we should log an error message, but still try to > >> continue with disabling the subchannel. What do you or others think? > > > > Logging an error may be useful (it's really fouled up at that time), but... > > > >> > >> > >> > >> > >>>> > >>>>>> + flush_workqueue(vfio_ccw_work_q); > >>>>>> + spin_lock_irq(sch->lock); > >>>>>> ret = cio_disable_subchannel(sch); > > > > ...there's a good chance that we'd get -EBUSY here, which would keep us > > in the loop. We probably need to break out after we got -EIO from > > cancel_halt_clear, regardless of which return code we get from the > > disable. > > Okay, for EIO we can log an error message and break out of the loop. > > I will send a v3. Are you going to queue patch 1 or patch 3 soon? If you > are then I will just send this patch separately. Yes, please do send it separately. I'm currently testing patch 1 and 3 on top of my patchset, will queue either with or without the halt/clear patches proper, depending on how soon I get acks/r-bs (hint, hint :) > > Thanks > Farhan > > > > > (It will be "interesting" to see what happens with such a stuck > > subchannel in the calling code; but I don't really see many options. > > Panic seems way too strong; maybe mark the subchannel as "broken; no > > idea how to fix"? But that would be a follow-on patch; I think if we > > avoid the endless loop here, this patch is a real improvement and > > should just go in.) > > > >>>>>> } while (ret == -EBUSY); > >>>>>> out_unlock: > >>>>> > >>>>> > >>>> > >>> > >>> > >> > > > > >