From: Geert Uytterhoeven <geert@linux-m68k.org> To: "Daniel W. S. Almeida" <dwlsalmeida@gmail.com> Cc: "mchehab+huawei@kernel.org" <mchehab+huawei@kernel.org>, "r.verdejo@samsung.com" <r.verdejo@samsung.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "nicolas@ndufresne.ca" <nicolas@ndufresne.ca>, "linux-kernel-mentees@lists.linuxfoundation.org" <linux-kernel-mentees@lists.linuxfoundation.org>, "linux-media@vger.kernel.org" <linux-media@vger.kernel.org> Subject: Re: [Linux-kernel-mentees] [v10 3/4] media: vidtv: add a bridge driver Date: Tue, 15 Sep 2020 15:35:00 +0200 [thread overview] Message-ID: <CAMuHMdVfoQ-vW3VeO8vDVALNREBV1afaSnwnc0jnkoXmvw6xhQ@mail.gmail.com> (raw) In-Reply-To: <EA89ED5D-E477-4464-B523-D729796CE0D0@getmailspring.com> Hi Daniel, On Tue, Sep 15, 2020 at 3:26 PM Daniel W. S. Almeida <dwlsalmeida@gmail.com> wrote: > >> + 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. All divisions involving 64-bit data are expensive, especially on 32-bit platforms. That's why we have the helpers in <linux/math.h>. Most of them implement simplified variants, which are less expensive. > >> + 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 > > <linux/math64.h>. > > > > 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, looking forward to it! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
next prev parent reply other threads:[~2020-09-15 13:35 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-08-21 12:58 [Linux-kernel-mentees] [v10 0/4] media: vidtv: Implement a virtual DVB driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] [v10 1/4] media: vidtv: implement a tuner driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] [v10 2/4] media: vidtv: implement a demodulator driver Daniel W. S. Almeida 2020-08-21 12:58 ` [Linux-kernel-mentees] [v10 3/4] media: vidtv: add a bridge driver Daniel W. S. Almeida 2020-09-15 11:53 ` Geert Uytterhoeven 2020-09-15 13:26 ` Daniel W. S. Almeida 2020-09-15 13:35 ` Geert Uytterhoeven [this message] 2020-09-15 18:13 ` Daniel W. S. Almeida 2020-09-16 7:01 ` Mauro Carvalho Chehab 2020-08-21 12:58 ` [Linux-kernel-mentees] [v10 4/4] media: Documentation: vidtv: Add ReST documentation for vidtv Daniel W. S. Almeida 2020-09-11 8:02 ` [Linux-kernel-mentees] [v10 0/4] media: vidtv: Implement a virtual DVB driver Mauro Carvalho Chehab 2020-09-11 12:18 ` Daniel W. S. Almeida 2020-09-11 13:10 ` Mauro Carvalho Chehab 2020-09-11 13:56 ` Mauro Carvalho Chehab 2020-09-12 2:54 ` Daniel W. S. Almeida 2020-09-12 7:38 ` Mauro Carvalho Chehab 2020-09-12 8:21 ` Hans Verkuil 2020-09-12 9:15 ` Mauro Carvalho Chehab 2020-09-12 14:49 ` Daniel W. S. Almeida 2020-09-12 17:57 ` Mauro Carvalho Chehab 2020-09-14 8:33 ` Hans Verkuil 2020-09-12 8:35 ` Hans Verkuil
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAMuHMdVfoQ-vW3VeO8vDVALNREBV1afaSnwnc0jnkoXmvw6xhQ@mail.gmail.com \ --to=geert@linux-m68k.org \ --cc=dwlsalmeida@gmail.com \ --cc=linux-kernel-mentees@lists.linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab+huawei@kernel.org \ --cc=nicolas@ndufresne.ca \ --cc=r.verdejo@samsung.com \ --subject='Re: [Linux-kernel-mentees] [v10 3/4] media: vidtv: add a bridge driver' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).