From: "Chickles, Derek" <Derek.Chickles@cavium.com>
To: Nicholas Mc Guire <hofrat@osadl.org>
CC: "Burla, Satananda" <Satananda.Burla@cavium.com>,
        "Manlunas, Felix" <Felix.Manlunas@cavium.com>,
        "Vatsavayi, Raghu" <Raghu.Vatsavayi@cavium.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] liquidio CN23XX: make timeout HZ independent
Thread-Topic: [PATCH] liquidio CN23XX: make timeout HZ independent
Thread-Index: AQHSV3OpJiM35Hk1vEWJrtE+ZHgWBqEK/4aQ
Date: Fri, 16 Dec 2016 20:08:04 +0000
Message-ID: <MWHPR07MB294310204EE20FC19B8E8612F39C0@MWHPR07MB2943.namprd07.prod.outlook.com>
References: <1481875839-30860-1-git-send-email-hofrat@osadl.org>
In-Reply-To: <1481875839-30860-1-git-send-email-hofrat@osadl.org>
Accept-Language: en-US
Content-Language: en-US
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Dec 2016 20:08:04.5020
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR07MB3182
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uBGK8KMp021422
Content-Length: 1833
Lines: 46

> -----Original Message-----
> From: Nicholas Mc Guire [mailto:hofrat@osadl.org]
> Sent: Friday, December 16, 2016 12:11 AM
> To: Chickles, Derek
> Cc: Burla, Satananda; Manlunas, Felix; Vatsavayi, Raghu;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Nicholas Mc Guire
> Subject: [PATCH] liquidio CN23XX: make timeout HZ independent
> 
> schedule_timeout_* takes a timeout in jiffies but the code currently is
> passing in a constant which makes this timeout HZ dependent, so pass it
> through msecs_to_jiffies() to fix this up.
> 
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> ---
> 
> Problem found by coccinelle spatch
> 
> The current delay can vary by a factor 10 depending on the HZ
> setting chose, which does not seem reasonable here.
> 
> The below patch sets the timeout to 10ms - it is though not clear
> if this is the intent or if it should be longer/shorter as it is not
> clear what HZ setting was assumed during design and used for testing.
> 
> This needs an ack by someone who knows the device and can confirm that
> 10ms is reasonable to wait for completion of queuing.

We were actually looking at this in parallel already to speed up driver
loading. It would be better if we changed LIO_MBOX_WRITE_WAIT_TIME
to 1 and applied the msecs_to_jiffies() to the line specified in your patch
and the loop just above that.

        while (readq(mbox->mbox_write_reg) != OCTEON_PFVFSIG) {
                schedule_timeout_uninterruptible(LIO_MBOX_WRITE_WAIT_TIME);
                if (count++ == LIO_MBOX_WRITE_WAIT_CNT) {
                        ret = OCTEON_MBOX_STATUS_FAILED;
                        break;
                }
        }       

If you can provide a new patch with both the changes we'll sign off on it.
Otherwise, it's on our list and we'll submit it soon ourselves.

Thanks,
Derek