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=-3.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 095B9C433E2 for ; Wed, 16 Sep 2020 00:36:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C051C207DE for ; Wed, 16 Sep 2020 00:36:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="rrUc/Nra" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727176AbgIPAgF (ORCPT ); Tue, 15 Sep 2020 20:36:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52672 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726634AbgION1R (ORCPT ); Tue, 15 Sep 2020 09:27:17 -0400 Received: from mail-qk1-x741.google.com (mail-qk1-x741.google.com [IPv6:2607:f8b0:4864:20::741]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DD14DC0611C2; Tue, 15 Sep 2020 06:26:23 -0700 (PDT) Received: by mail-qk1-x741.google.com with SMTP id t138so4195164qka.0; Tue, 15 Sep 2020 06:26:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:content-transfer-encoding:content-disposition; bh=ECv1tH4dixNsdKxQCqvGJD0xyYRqc/3rORp/BHVcsQ4=; b=rrUc/Nra5A278GUSmFmQgG0+9yiyz/Kp+IF8tT6eTd3T2zNG1ooEzO9ZlSCCOlPi9X hG+YWCMJqq8/nKJTh+UiUyP6E0SMGLppdAPCAjaCaYUxKK9DFJQnilOVBKn6ywicFDW9 T/ep8wEpWgIA4CJKCzGviWoNMJFKt7DlgQ9U8gBz4HvwOmAkzWa1iRRXhtlTfNTo8kYK 0Hh34hrXFgxJGrpV+bUnZYlVeXVlXR61XN5mr9hcMvIBijH7CUpZH5Ym59hVq3kWILQU DSxDsdz+dODQ0fV5f6teQgCvV8Lb7WfrlqTz0PghqhgX0cWebwp42woWsJ9gpR9iVoNM iUXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version:content-transfer-encoding :content-disposition; bh=ECv1tH4dixNsdKxQCqvGJD0xyYRqc/3rORp/BHVcsQ4=; b=Ju0QQeeXlNAkiUqnCFHgoU3oP5xMqJDNV+1x7BzbtboOmS+M5Z157hxVOS9SAFo7IH iR4gr4W7wVyHOy0jQ71E71c1JRXtdhM3wE5xhoaDr3wx+52Vct7RpqUSbnx1JgSkptap LWxEn/RjRaNz9OLtQEHISvb6V77yOfNcNZOuk9hlSF0XGyZ1BRbEeHomQg6Idho+0LdU EzAFNYvHZaKiBZKnukRDsZQ56bosvd/odS4joSmlmumruqJAADHxpPNvlYeH7mtXv3Vm pwi1x+hS25GPyhKRRpgeXnB+QxNm1L9eyc2heTEey/fAJ/OefUxiL2cumsbE/Eff/ukQ f9Cg== X-Gm-Message-State: AOAM531ggIQXjc39GoT9KFwRBaG9DVRfb/5rJXo+xdQ5XoVvLoZehUqK Ff/l5IMh1qgZjDlRtsfRd8g= X-Google-Smtp-Source: ABdhPJzuz0o4wLDw+Cjo2IgGvOxLPSDN8JBtV21hG89SBTjGoXxWC7vpcN1/qhgD+HtvnbTR9wKs5w== X-Received: by 2002:a05:620a:13e8:: with SMTP id h8mr17496044qkl.322.1600176383031; Tue, 15 Sep 2020 06:26:23 -0700 (PDT) Received: from dwls-dell ([2804:14d:72b1:8920:a2ce:f815:f14d:bfac]) by smtp.gmail.com with ESMTPSA id g45sm17473139qtb.60.2020.09.15.06.26.20 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Sep 2020 06:26:22 -0700 (PDT) Date: Tue, 15 Sep 2020 10:26:17 -0300 From: "Daniel W. S. Almeida" To: Geert Uytterhoeven Cc: "=?utf-8?Q?mchehab+huawei=40kernel.org?=" , "=?utf-8?Q?r.verdejo=40samsung.com?=" , "=?utf-8?Q?nicolas=40ndufresne.ca?=" , "=?utf-8?Q?linux-media=40vger.kernel.org?=" , "=?utf-8?Q?skhan=40linuxfoundation.org?=" , "=?utf-8?Q?linux-kernel-mentees=40lists.linuxfoundation.org?=" , "=?utf-8?Q?linux-kernel=40vger.kernel.org?=" Message-ID: In-Reply-To: References: Subject: Re: [v10 3/4] media: vidtv: add a bridge driver X-Mailer: Mailspring MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Geert, Thanks for bringing this to my attention. >> + u32 nbytes = 0; /* the number of bytes written by this function */ >> + >> + u64 nbytes_expected; /* the number of bytes we should have written */ >> + u64 nbytes_streamed; /* the number of bytes we actually wrote */ >> + u32 num_null_pkts; /* number of null packets to bridge the gap */ >> + >> + u64 elapsed_time_msecs = jiffies_to_usecs(m->timing.current_jiffies - >> + m->timing.past_jiffies); >> + >> + elapsed_time_msecs = min(elapsed_time_msecs, >> (u64)VIDTV_MAX_SLEEP_USECS / 1000); >> + nbytes_expected = div64_u64(m->mux_rate_kbytes_sec * 1000, MSEC_PER_SEC); > > Seriously?!? > > You multiply by 1000 first, followed by a division by 1000 using an > expensive 64-by-64 division? This entire function is broken and needs a do-over :) > using an expensive 64-by-64 division? I am new to kernel development. I wasn't even aware that this was expensive, to be honest. >> + if (nbytes_streamed < nbytes_expected) { >> + /* can't write half a packet: roundup to a 188 multiple */ >> + nbytes_expected = roundup(nbytes_expected - nbytes_streamed, TS_PACKET_LEN); > > drivers/media/test-drivers/vidtv/vidtv_mux.o: In function `vidtv_mux_tick': > vidtv_mux.c:(.text+0x788): undefined reference to `__udivdi3' > > This is a 64-by-32 division, hence it should use a helper from > . > > However, I'm wondering if "nbytes_expected - nbytes_streamed" is > guaranteed to be a "small" number, hence a 32-by-32 division would be > sufficient? I think so. I will send a patch to address the things you pointed out in this email. -- thanks -- Daniel 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=-3.3 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no 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 1A7B7C43461 for ; Tue, 15 Sep 2020 13:26:29 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 7055022240 for ; Tue, 15 Sep 2020 13:26:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="rrUc/Nra" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7055022240 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0E35285821; Tue, 15 Sep 2020 13:26:28 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lAWrdDWwyC-h; Tue, 15 Sep 2020 13:26:27 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id A2599856CB; Tue, 15 Sep 2020 13:26:27 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9EEB3C0864; Tue, 15 Sep 2020 13:26:27 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 94186C0051 for ; Tue, 15 Sep 2020 13:26:26 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 88BE8204D4 for ; Tue, 15 Sep 2020 13:26:26 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id RmMpDy9N0SaH for ; Tue, 15 Sep 2020 13:26:24 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-qk1-f193.google.com (mail-qk1-f193.google.com [209.85.222.193]) by silver.osuosl.org (Postfix) with ESMTPS id 21D42204F0 for ; Tue, 15 Sep 2020 13:26:24 +0000 (UTC) Received: by mail-qk1-f193.google.com with SMTP id o5so4091222qke.12 for ; Tue, 15 Sep 2020 06:26:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:message-id:in-reply-to:references:subject :mime-version:content-transfer-encoding:content-disposition; bh=ECv1tH4dixNsdKxQCqvGJD0xyYRqc/3rORp/BHVcsQ4=; b=rrUc/Nra5A278GUSmFmQgG0+9yiyz/Kp+IF8tT6eTd3T2zNG1ooEzO9ZlSCCOlPi9X hG+YWCMJqq8/nKJTh+UiUyP6E0SMGLppdAPCAjaCaYUxKK9DFJQnilOVBKn6ywicFDW9 T/ep8wEpWgIA4CJKCzGviWoNMJFKt7DlgQ9U8gBz4HvwOmAkzWa1iRRXhtlTfNTo8kYK 0Hh34hrXFgxJGrpV+bUnZYlVeXVlXR61XN5mr9hcMvIBijH7CUpZH5Ym59hVq3kWILQU DSxDsdz+dODQ0fV5f6teQgCvV8Lb7WfrlqTz0PghqhgX0cWebwp42woWsJ9gpR9iVoNM iUXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:message-id:in-reply-to :references:subject:mime-version:content-transfer-encoding :content-disposition; bh=ECv1tH4dixNsdKxQCqvGJD0xyYRqc/3rORp/BHVcsQ4=; b=EtyZesMhcRJgClNGIc+JaOnXYhiF0Py6xT1R2STek5P76zZj8vs/Gheeq+S6HzKQoI AogBlHB7dHLeu7IiE3M38JTSMbZ8XHRoM9UYVnKnfsp2PfEezSnUfqNRjrNPKGOxrW6P iN9QerJ/EgTo8b8GCwQi2Yvcy93DOUl4ih/F4BzCZKs+stz7gobd4wXP9TgppJ/k56Xo X8KxLL7G5hsDKZmVIxjnb80l/YUqTSqZN5QiRu16Hgf6TzryTU7X5buDRV2KTszFFouu f+RIKnPZLDjYIEntMHlgWHLRH9Ce09n+auM/TMsHSUA7QjpspvB/lav2qQ9DuSxpAtH1 y/pQ== X-Gm-Message-State: AOAM532Bdum0/OPS31++A3jZMNHxgGsUjLq6KJq2oqCCcFfhOv7O+yJL j/RhjSq647RMAi6o1jxchMU= X-Google-Smtp-Source: ABdhPJzuz0o4wLDw+Cjo2IgGvOxLPSDN8JBtV21hG89SBTjGoXxWC7vpcN1/qhgD+HtvnbTR9wKs5w== X-Received: by 2002:a05:620a:13e8:: with SMTP id h8mr17496044qkl.322.1600176383031; Tue, 15 Sep 2020 06:26:23 -0700 (PDT) Received: from dwls-dell ([2804:14d:72b1:8920:a2ce:f815:f14d:bfac]) by smtp.gmail.com with ESMTPSA id g45sm17473139qtb.60.2020.09.15.06.26.20 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Sep 2020 06:26:22 -0700 (PDT) Date: Tue, 15 Sep 2020 10:26:17 -0300 From: "Daniel W. S. Almeida" To: Geert Uytterhoeven Message-ID: In-Reply-To: References: X-Mailer: Mailspring MIME-Version: 1.0 Content-Disposition: inline Cc: "=?utf-8?Q?mchehab+huawei=40kernel.org?=" , "=?utf-8?Q?r.verdejo=40samsung.com?=" , "=?utf-8?Q?linux-kernel=40vger.kernel.org?=" , "=?utf-8?Q?nicolas=40ndufresne.ca?=" , "=?utf-8?Q?linux-kernel-mentees=40lists.linuxfoundation.org?=" , "=?utf-8?Q?linux-media=40vger.kernel.org?=" Subject: Re: [Linux-kernel-mentees] [v10 3/4] media: vidtv: add a bridge driver X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 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 Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" Hi Geert, Thanks for bringing this to my attention. >> + u32 nbytes = 0; /* the number of bytes written by this function */ >> + >> + u64 nbytes_expected; /* the number of bytes we should have written */ >> + u64 nbytes_streamed; /* the number of bytes we actually wrote */ >> + u32 num_null_pkts; /* number of null packets to bridge the gap */ >> + >> + u64 elapsed_time_msecs = jiffies_to_usecs(m->timing.current_jiffies - >> + m->timing.past_jiffies); >> + >> + elapsed_time_msecs = min(elapsed_time_msecs, >> (u64)VIDTV_MAX_SLEEP_USECS / 1000); >> + nbytes_expected = div64_u64(m->mux_rate_kbytes_sec * 1000, MSEC_PER_SEC); > > Seriously?!? > > You multiply by 1000 first, followed by a division by 1000 using an > expensive 64-by-64 division? This entire function is broken and needs a do-over :) > using an expensive 64-by-64 division? I am new to kernel development. I wasn't even aware that this was expensive, to be honest. >> + if (nbytes_streamed < nbytes_expected) { >> + /* can't write half a packet: roundup to a 188 multiple */ >> + nbytes_expected = roundup(nbytes_expected - nbytes_streamed, TS_PACKET_LEN); > > drivers/media/test-drivers/vidtv/vidtv_mux.o: In function `vidtv_mux_tick': > vidtv_mux.c:(.text+0x788): undefined reference to `__udivdi3' > > This is a 64-by-32 division, hence it should use a helper from > . > > However, I'm wondering if "nbytes_expected - nbytes_streamed" is > guaranteed to be a "small" number, hence a 32-by-32 division would be > sufficient? I think so. I will send a patch to address the things you pointed out in this email. -- thanks -- Daniel _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees