Re: [PATCH] vdso/gettimeofday: Reload sequence counter after switch to time page in do_aux()

[Ricardo Ribalda]

Hi Thomas

On Mon, 27 Apr 2026 at 14:27, Thomas Weißschuh
<thomas.weissschuh@xxxxxxxxxxxxx> wrote:
>
> Hi Ricardo,
>
> On Mon, Apr 27, 2026 at 08:09:49PM +0800, Ricardo Ribalda wrote:
> > Thanks for the patch. Any idea when do you plan to land this?
>
> Thomas Gleixner will pick it up as soon as he gets to it.
> That is if he has no objections to the patch itself.
> I trust that it will be fairly soon.
>
> Can you confirm that the patch fixes the warning you reported originally?

It did indeed fix the issue:
https://gitlab.freedesktop.org/linux-media/users/ci/-/pipelines/1654482/test_report?job_name=cross-gcc

Tested-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>

>
>
> Thomas
>
> > On Wed, 22 Apr 2026 at 11:49, Christophe Leroy (CS GROUP)
> > <chleroy@xxxxxxxxxx> wrote:
> > >
> > >
> > >
> > > Le 22/04/2026 à 11:42, Thomas Weißschuh a écrit :
> > > > After switching to the real data pages, the sequence counter needs to be
> > > > reloaded from there. The code using vdso_read_begin_timens() assumed
> > > > this worked by 'continue' jumping to the *beginning* of the do-while
> > > > retry loop. However the 'continue' jumps to the *end* of said loop,
> > > > evaluating the exit condition. If the data page has a sequence counter
> > > > of '1' it will match the one from the time namespace page and prematurely
> > > > exit the retry loop. This would result in garbage returned to the caller.
> > > >
> > > > Reload the sequence counter after switching the pages by using an inner
> > > > while loop again, which will loop at most once.
> > > >
> > > > The loop generates slightly better code than an explicit reload through
> > > > 'seq = vdso_read_begin()'.
> > > >
> > > > Reported-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > > > Closes: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FCANiDSCsOy0P1if-gJZqOM5pTJ0RDcwVfru1B7KFbTOEMqjPKJw%40mail.gmail.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C8090d715bf644286a5e908dea0538801%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C639124477796711565%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=yQti2tvfaExxkn%2BoD7wzPhAkFTYQ7Sr%2FV3e3fRdJYVY%3D&reserved=0
> > > > Fixes: ed78b7b2c5ae ("vdso/gettimeofday: Add a helper to read the sequence lock of a time namespace aware clock")
> > > > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
> > >
> > > Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@xxxxxxxxxx>
> > >
> > > > ---
> > > >   lib/vdso/gettimeofday.c | 5 ++---
> > > >   1 file changed, 2 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/lib/vdso/gettimeofday.c b/lib/vdso/gettimeofday.c
> > > > index a5798bd26d20..da224011fafd 100644
> > > > --- a/lib/vdso/gettimeofday.c
> > > > +++ b/lib/vdso/gettimeofday.c
> > > > @@ -248,11 +248,10 @@ bool do_aux(const struct vdso_time_data *vd, clockid_t clock, struct __kernel_ti
> > > >       vc = &vd->aux_clock_data[idx];
> > > >
> > > >       do {
> > > > -             if (vdso_read_begin_timens(vc, &seq)) {
> > > > +             while (vdso_read_begin_timens(vc, &seq)) {
> > > > +                     /* Re-read from the real time data page, reload seq by looping */
> > > >                       vd = __arch_get_vdso_u_timens_data(vd);
> > > >                       vc = &vd->aux_clock_data[idx];
> > > > -                     /* Re-read from the real time data page */
> > > > -                     continue;
> > > >               }
> > > >
> > > >               /* Auxclock disabled? */
> > > >
> > > > ---
> > > > base-commit: aa70a22882fe51fadd2392b480864d15e1b9ab3d
> > > > change-id: 20260422-vdso-aux-timens-loop-acbaa5a2434e
> > > >
> > > > Best regards,
> > > > --
> > > > Thomas Weißschuh <thomas.weissschuh@xxxxxxxxxxxxx>
> > > >
> > >
> >
> >
> > --
> > Ricardo Ribalda



-- 
Ricardo Ribalda