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=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 356B4C433EF for ; Wed, 15 Sep 2021 14:20:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1598C6113E for ; Wed, 15 Sep 2021 14:20:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233945AbhIOOVY (ORCPT ); Wed, 15 Sep 2021 10:21:24 -0400 Received: from mout.kundenserver.de ([212.227.126.135]:35467 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233771AbhIOOVT (ORCPT ); Wed, 15 Sep 2021 10:21:19 -0400 Received: from mail-wm1-f50.google.com ([209.85.128.50]) by mrelayeu.kundenserver.de (mreue009 [213.165.67.97]) with ESMTPSA (Nemesis) id 1N8GIa-1mvZHz3iDo-014A92 for ; Wed, 15 Sep 2021 16:19:59 +0200 Received: by mail-wm1-f50.google.com with SMTP id l18-20020a05600c4f1200b002f8cf606262so4951647wmq.1 for ; Wed, 15 Sep 2021 07:19:59 -0700 (PDT) X-Gm-Message-State: AOAM532WhDXrhJnlYjjy10Z76RYCJ1M2em6pRUXiJLQ1D8R+O1lVlsnh pGO8VrQaEKHQ1j7Mpdic/3OVtaVqWagseskltBA= X-Google-Smtp-Source: ABdhPJz8J4pJD+afwfweySjUuRBGa8LE2pnN/Ho6Oq2xiKUYXAx1VWdT2VBNYglVJPI9KXcGv93s3eNUwo2NtweqBfs= X-Received: by 2002:a05:600c:896:: with SMTP id l22mr4757847wmp.173.1631715599598; Wed, 15 Sep 2021 07:19:59 -0700 (PDT) MIME-Version: 1.0 References: <20210914213532.396654-1-gascoar@gmail.com> <260b38b8-6f3f-f6cc-0388-09a269ead507@i2se.com> In-Reply-To: From: Arnd Bergmann Date: Wed, 15 Sep 2021 16:19:43 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/8] staging: vchiq_arm: replace sleep() with usleep_range() To: Phil Elwell Cc: Stefan Wahren , Gaston Gonzalez , linux-staging@lists.linux.dev, gregkh , Nicolas Saenz Julienne , Arnd Bergmann , Dan Carpenter , ojaswin98@gmail.com, Amarjargal Gundjalam , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" , Linux ARM , bcm-kernel-feedback-list , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:L+ofbII3p2/T/1lYlMroQOLAhrLlzjCQ9vxgucNQ/daKiibn5Ff Rq17qeNBi1eYmg00eBqWiyLZjR27MfQoAPQ12/jthth3wZtDEOZi71iCKZfEEnljpGOzwrp Z1zZv5XHGX63zB9UM+9xVHO78a1l1dVIqGAHv5rePc5pf4jR/HXBe2GGFX0De0is0szmaWQ I6LAP2osynLfUe3Ft15zw== X-UI-Out-Filterresults: notjunk:1;V03:K0:M+LpcZn8iiQ=:VcjfrkdrQEEq1KGbugyelZ bn9jKDL/zjtRs3VjKKgVpsLP7mDbFqGman2v7cKyc0HY9NVuy5KWStjN4C/EW0YSaWqN2uRED k8cmpvdbILdoBsa+DPybAeOIysNz0UddU9RRXkvoTYHIUhA5FEVoI0vQFqVe54Mlql9J/PYea k3shewmIJTTWa3z/M5+pE7Jsd2zwrqt8AtRWPuMIdYvA7/8X6b7AeOu18LWo0FWJWGraJ47o3 E15kFWVCkaE17kl5qixaj5aMVdhbgLDcM49TS38N9qjnYbVXJ8WnbTwM/wFb+LuqXJXsoN+ZT LNHK1h37zxwyG79SAwYkjfIK9xU9U6vDiyyXl3b3Ye5sYZNrYHpuBkxfjScrz2fcNNN0tH6ca VzjXi6AauGelBzZLYZpFr6J2qjaMw24Xmyz5kVxlnT78p1xrmeBgrpNPeaFnjMMB3+nYBA3Ni PEGssxQWEbnqMAeZm+hgbfHpTGvCIRGDgKyXsWWrh0WwXZFhN//7FKKE5R9ftoEsYoIofkS+R htN0CtebhpyLxdi94ne+qk81raafzDZAw9POPMbhoR1xCGFVYRMEhUt52DdwIFwd4pSZa64H9 w8cDT14XxTiqs1iGYBtNodfb/t6sI3nR5RccMhaWeeFjpQkjkBzbsS+rUHOdJgwduweQq+LQv zU/HVjNUUwCob8s7U/GR2/IgG+qmz5y6J+qZKOUUlGrA9PylQDUG3wa4F+jSvVrxjDBq+wFO9 qyVz25191XpRsId3Tf0SO1HFs+xtWEdQ9Z2gPTIz6xRhkNEB+uyNobtohwYPMJ8duIL7fLqKv /N1e1j0a8o/JMPAp7QxDbwXU1esN0H4F/tdMKQ1tLOk54g6KApU8jmyM/WvGBEglHUCTehNIf uP8Ow1IuPpCmqiddlfyw== Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 15, 2021 at 10:06 AM Phil Elwell wrote: > > Hi Stefan, > > On 15/09/2021 06:21, Stefan Wahren wrote: > > Hi, > > > > Am 14.09.21 um 23:35 schrieb Gaston Gonzalez: > >> usleep_range() should be used instead of sleep() when sleepings range > >> from 10 us to 20 ms, [1]. > >> > >> Reported by checkpatch.pl > >> > >> [1] Documentation/timers/timers-howto.txt > >> --- > >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> index b25369a13452..0214ae37e01f 100644 > >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> @@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, > >> if (status != VCHIQ_RETRY) > >> break; > >> > >> - msleep(1); > >> + usleep_range(1000, 1100); > > > > from my understanding the usage of usleep_range() and hrtimers isn't > > necessary here. The intention is to sleep a little bit and not "exactly" > > 1 ms. > > > > @Phil Elwell: what is your opinion? > > Exactly - the aim is just to stop it spinning. This is usually a sign for something else being wrong in the thing it's waiting for. I can see multiple 'return VCHIQ_RETRY' statements in vchiq_bulk_transfer(), however these all happen when the task has received a signal and wait_for_completion_interruptible() or mutex_lock_killable() has returned an error. I don't see why one of them would return on any signal and the other one only fatal signals, as you usually want those conditions to be the same, but in either case, the loop is broken because as soon as you get a signal, those interfaces will keep returning the same error and you can never break out of the loop any more. I don't know how to properly fix it, but it's clear that vchiq_bulk_transmit() needs to propagate the -EINTR or -ERESTSTARTSYS back to user space to let the calling task handle the signal before retrying. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout.kundenserver.de (mout.kundenserver.de [212.227.126.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 668073FD2 for ; Wed, 15 Sep 2021 14:20:02 +0000 (UTC) Received: from mail-wm1-f44.google.com ([209.85.128.44]) by mrelayeu.kundenserver.de (mreue010 [213.165.67.97]) with ESMTPSA (Nemesis) id 1MeCd5-1n0g471NHL-00bIG2 for ; Wed, 15 Sep 2021 16:20:00 +0200 Received: by mail-wm1-f44.google.com with SMTP id k5-20020a05600c1c8500b002f76c42214bso4944014wms.3 for ; Wed, 15 Sep 2021 07:19:59 -0700 (PDT) X-Gm-Message-State: AOAM530XePu+l9eT0uFPDaEbO0SVVaaCWec/78Hw9h9dM5Z+nitiFvuc pj2gZ+0arembua8MgGGs9RhSWt7M6KpkYaaq3Co= X-Google-Smtp-Source: ABdhPJz8J4pJD+afwfweySjUuRBGa8LE2pnN/Ho6Oq2xiKUYXAx1VWdT2VBNYglVJPI9KXcGv93s3eNUwo2NtweqBfs= X-Received: by 2002:a05:600c:896:: with SMTP id l22mr4757847wmp.173.1631715599598; Wed, 15 Sep 2021 07:19:59 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-staging@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20210914213532.396654-1-gascoar@gmail.com> <260b38b8-6f3f-f6cc-0388-09a269ead507@i2se.com> In-Reply-To: From: Arnd Bergmann Date: Wed, 15 Sep 2021 16:19:43 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/8] staging: vchiq_arm: replace sleep() with usleep_range() To: Phil Elwell Cc: Stefan Wahren , Gaston Gonzalez , linux-staging@lists.linux.dev, gregkh , Nicolas Saenz Julienne , Arnd Bergmann , Dan Carpenter , ojaswin98@gmail.com, Amarjargal Gundjalam , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" , Linux ARM , bcm-kernel-feedback-list , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Provags-ID: V03:K1:sIEfIE7Gnk3aKfR5qC2Jie8MhH4JhL6QzeY6TsEHch0cjkRphK0 o10RETQzs3f1odTHaKAjuJwMFqdvqwLPQdZJhFi9qEWo/pWeBRdi81WcDrDQriEsIVRtu8l unOJ0lF9n7lKC6AM+WLxgR8eyExDmTlq6kvat6KG/SEfRYxZoWd44R2jl7ki5k3j8oWjP+G 9/hvwYpY/Ihq3wJ4J2bdw== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1;V03:K0:C2lxkHMm7GA=:5Xbfm6n1KxSJ+X4L5yXF7I 6P4gXT/NcNB6BBBZPJhO40oh6IMGkN5QcaocJgSzCv+AjfH25fmCxXiM/ams6gT5hqho0xDUs xq3HiIcqOUla+ioyLUyALFabQ5h7tuPnyqEowD/6spmdyEkwr3zHhhnPMrRnoEg97NYVGtE9/ u80RKeVFyqXqNv41PO/mwOZpmVO7AU7ZfRRDwLNR+7QiwrXe13nyNwKWhsK4I4YeNUTIiCjCB LJuc8g2O4CYNVJBQ5ob28eYC0zxSzD10SCTQsbYQ5G8uP9P59XeUvKvMc6hN7q01x9Kpt0wjG D1bJHCbi3ROXIq7MD8Dk9/XbTIUtn0SYhl05390h68X6FAhRTVpH/oRWLwaloyNEXsxURstEr 1mL8Va4hb/RAKBPlyvxZLYOfGkygrI9ARgs3XPhrnZkodM9lXM/lR5IJ/ksje4+yPwktKvKmt Z1gW1VKwYzoMmokjWeGlFN2dItEvLX0CGJMa1fOVmq7HlmoZxO5Fs1yeApFFShZMk3vc4wa7G 0hGfLmLxRO04pYQ1XBqmoAQ8C2t4EPxM1IYM57DLAfnYTAPB4fu+MLeGEYHFJl0eIfrVALBUQ sphIUbToLDZK5y1IBckZefuPbxCJJf+nW09CKjY8EEp4uehanyXHhocQA1POB8GCyYHYoX7SO JX6gOu4USZLFpwVrwDXObbShEttqFzU4d6hdhUciH0MFVYcGhvRnv5WsgQ40eVm6h5KOJYMBm ZiJB0yrf5kfQypF7odRigUkyHGcdD7H+25DBLOHy65b4YJgi9sLVP1ZEKr4EXQ5kgPyXwXJOt RhwmU3/FsB4na7nKtYOF85nqPIQEtlaFGDaw1lBX8TZf3Z8g9XjbIGNLVqus+U7TltH1yZMLW 8I4cnpxm4S0TAKi4u3XQ== On Wed, Sep 15, 2021 at 10:06 AM Phil Elwell wrote: > > Hi Stefan, > > On 15/09/2021 06:21, Stefan Wahren wrote: > > Hi, > > > > Am 14.09.21 um 23:35 schrieb Gaston Gonzalez: > >> usleep_range() should be used instead of sleep() when sleepings range > >> from 10 us to 20 ms, [1]. > >> > >> Reported by checkpatch.pl > >> > >> [1] Documentation/timers/timers-howto.txt > >> --- > >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> index b25369a13452..0214ae37e01f 100644 > >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> @@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, > >> if (status != VCHIQ_RETRY) > >> break; > >> > >> - msleep(1); > >> + usleep_range(1000, 1100); > > > > from my understanding the usage of usleep_range() and hrtimers isn't > > necessary here. The intention is to sleep a little bit and not "exactly" > > 1 ms. > > > > @Phil Elwell: what is your opinion? > > Exactly - the aim is just to stop it spinning. This is usually a sign for something else being wrong in the thing it's waiting for. I can see multiple 'return VCHIQ_RETRY' statements in vchiq_bulk_transfer(), however these all happen when the task has received a signal and wait_for_completion_interruptible() or mutex_lock_killable() has returned an error. I don't see why one of them would return on any signal and the other one only fatal signals, as you usually want those conditions to be the same, but in either case, the loop is broken because as soon as you get a signal, those interfaces will keep returning the same error and you can never break out of the loop any more. I don't know how to properly fix it, but it's clear that vchiq_bulk_transmit() needs to propagate the -EINTR or -ERESTSTARTSYS back to user space to let the calling task handle the signal before retrying. Arnd 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=-9.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 E5DBBC433EF for ; Wed, 15 Sep 2021 14:21:38 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A6E4560F11 for ; Wed, 15 Sep 2021 14:21:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A6E4560F11 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arndb.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iwjb4FV3BU5ppnEgDyqzacJRBMADzdh4GuYHvynJ5z0=; b=MnX0evTsZBy5He e+42j9v43unMZwiHgB5nHjuPcopq84nBu2jhsaYeR7DFKG5ItKBmvlpRXgYrhxrHzzrWi5d0UYCff 4k8J+79YEszedQkmJqFzyftaQZDKfUULKVd1Bx9S7ix3X35+9Tq/f+c/wb8P0AQ1BB05x0lpknGuo bCDOvvzd87Pre+7d949ycMjh56O/+1GY/0gFva2O2jlu16rnpxqYRtddtXoson8cUA1tluMj8VrO0 IdNFazNfQvkis6d0HZf+xRFb4AWGw6uJBa9sdm9SQbr2k6UMgY5eDU716MGd2DwBhnGaMRacBY49g Pye+s01S3Q9TUXiN85tg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mQVlh-009J65-Hs; Wed, 15 Sep 2021 14:20:09 +0000 Received: from mout.kundenserver.de ([212.227.17.24]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mQVlc-009J4l-Tn; Wed, 15 Sep 2021 14:20:06 +0000 Received: from mail-wm1-f52.google.com ([209.85.128.52]) by mrelayeu.kundenserver.de (mreue109 [213.165.67.113]) with ESMTPSA (Nemesis) id 1Mw9Lu-1mjQCC0k4K-00s60u; Wed, 15 Sep 2021 16:20:00 +0200 Received: by mail-wm1-f52.google.com with SMTP id 192-20020a1c04c9000000b002f7a4ab0a49so4017405wme.0; Wed, 15 Sep 2021 07:20:00 -0700 (PDT) X-Gm-Message-State: AOAM530rmP9icsD2CfwdHiDlioS66dwLHx5in8dfsC85mbF17AAWG+dR JDQCE2IvEj8vxIBiuDWypShS2sj/kO1D7MlM6rU= X-Google-Smtp-Source: ABdhPJz8J4pJD+afwfweySjUuRBGa8LE2pnN/Ho6Oq2xiKUYXAx1VWdT2VBNYglVJPI9KXcGv93s3eNUwo2NtweqBfs= X-Received: by 2002:a05:600c:896:: with SMTP id l22mr4757847wmp.173.1631715599598; Wed, 15 Sep 2021 07:19:59 -0700 (PDT) MIME-Version: 1.0 References: <20210914213532.396654-1-gascoar@gmail.com> <260b38b8-6f3f-f6cc-0388-09a269ead507@i2se.com> In-Reply-To: From: Arnd Bergmann Date: Wed, 15 Sep 2021 16:19:43 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/8] staging: vchiq_arm: replace sleep() with usleep_range() To: Phil Elwell Cc: Stefan Wahren , Gaston Gonzalez , linux-staging@lists.linux.dev, gregkh , Nicolas Saenz Julienne , Arnd Bergmann , Dan Carpenter , ojaswin98@gmail.com, Amarjargal Gundjalam , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" , Linux ARM , bcm-kernel-feedback-list , Linux Kernel Mailing List X-Provags-ID: V03:K1:dRk0CB5ZYYMvm0Mqjdu0GWoNQESHPWRWQO7cge7BfbqfQZPIKk4 haE6SAQTS/CO3a+qK3RoMeqiCmsWWQGorDVdtkM6xV+ztVs5yUXyoNl6QGS3VayWtHVx6Uk cXMbyWHQXgDycQbCFKInYROyqfRkd0cqAC5HgHNOpoipMfUidldPnjCjczQzOz4Gj0bti/C buIgm7XQW0OrO3CLWysAw== X-UI-Out-Filterresults: notjunk:1;V03:K0:hwl/0inLWR8=:3wr0Z4W2+dvCxNkKU394Km TT40cplLtLK+1xB6TPLq9dDSqPydpHzpCc0tf+gpPm0jkPreHjL7jWOo2ViIrZzWDsGlPbN6j o6Ump4IB+Y0bIsymgWza0eGwYMbUGx2vyq3Euho7s/0xme08mXl7Wle9TKQ/bcJyfn/JtqJ76 y8chaho2r7ljNxpVSSXI7fFog/ptdsiX26d6ewYAspQ5mAvEr501XI+6v3wrbs8cQz7Yl2N63 7Pn4STJcT6OYfXMCcrhQrVpCVOYJRdtiAMYTYiGwQf3OxiHLSkbDYPV+FoasixyNAWuxXwivn cFzTE+NBC8ahRgrFSQxdkdNw7Mt8OB1QW3B8Wv+CVB6bXBm536WfqvUgWQRuMvBQZ2ih/882F qCNS6ZhpIdbVqGGuqRafS0fgi+xOH/k0pmwBnzoK+NxgSgpmteokC07uyHZhByH8R7Nsl6Gpb /+l9wgH/f+FGmHrrZRLKwPf6kbmgjsjkpjFLxVHVkFsZtrkCconSVoNSs6OABtZhEFVlLqvE8 OckGMrundRr/XtEpXjmulTFCVOoqiTQmuQgNc3nAxk/aKeWyyQwpeKEW5dwkiHPyeBcpbZNev xBUhMe3FipLlJn8SgTjrpGvVpJ4GaFDVLoH8qKnpU+t9ywyYnwEcZu23EJpCUrft2ay45QFJ5 NY8MRtHjkOtp1hVI/OZ5FogxH3wFVqIn3XIs9o1EAKpc7cTU2dE/XabMX5R97TsAI6a10Q1fn umkV5/JmA1Slit6T1VuOVUX5cqWAYK8DwlVMWOempFo4Cj+I2DgW3AkcHjwTKlDIZ45Ib8t/+ 5Axxhfl1p9xHV4Re/jPa0ulEpe9a/+l+MvaJdtyuYTXQY4HthFqoFbpGJswQWubN0XphvOOF9 YjnjNaicuuwW6UmoSSVg== X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210915_072005_305085_2BF570BB X-CRM114-Status: GOOD ( 25.19 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Sep 15, 2021 at 10:06 AM Phil Elwell wrote: > > Hi Stefan, > > On 15/09/2021 06:21, Stefan Wahren wrote: > > Hi, > > > > Am 14.09.21 um 23:35 schrieb Gaston Gonzalez: > >> usleep_range() should be used instead of sleep() when sleepings range > >> from 10 us to 20 ms, [1]. > >> > >> Reported by checkpatch.pl > >> > >> [1] Documentation/timers/timers-howto.txt > >> --- > >> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> index b25369a13452..0214ae37e01f 100644 > >> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > >> @@ -824,7 +824,7 @@ vchiq_bulk_transmit(unsigned int handle, const void *data, unsigned int size, > >> if (status != VCHIQ_RETRY) > >> break; > >> > >> - msleep(1); > >> + usleep_range(1000, 1100); > > > > from my understanding the usage of usleep_range() and hrtimers isn't > > necessary here. The intention is to sleep a little bit and not "exactly" > > 1 ms. > > > > @Phil Elwell: what is your opinion? > > Exactly - the aim is just to stop it spinning. This is usually a sign for something else being wrong in the thing it's waiting for. I can see multiple 'return VCHIQ_RETRY' statements in vchiq_bulk_transfer(), however these all happen when the task has received a signal and wait_for_completion_interruptible() or mutex_lock_killable() has returned an error. I don't see why one of them would return on any signal and the other one only fatal signals, as you usually want those conditions to be the same, but in either case, the loop is broken because as soon as you get a signal, those interfaces will keep returning the same error and you can never break out of the loop any more. I don't know how to properly fix it, but it's clear that vchiq_bulk_transmit() needs to propagate the -EINTR or -ERESTSTARTSYS back to user space to let the calling task handle the signal before retrying. Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel