On Mon, Jul 31, 2023 at 08:31:41PM -0500, Scott Cheloha wrote:
> On Mon, Jul 31, 2023 at 10:04:44PM +0200, Claudio Jeker wrote:
> > On Mon, Jul 31, 2023 at 09:49:30PM +0200, Claudio Jeker wrote:
> > > On Mon, Jul 31, 2023 at 08:03:41PM +0300, Vitaliy Makkoveev wrote:
> > > > This is the culprit:
> > > > 
> > > > schedule_timeout_uninterruptible(long timeout)
> > > > {
> > > >         tsleep(curproc, PWAIT, "schtou", timeout);
> > > >         return 0;
> > > > }
> > > > 
> > > 
> > > Please give this a try. I think on initialization
> > > intel_dp_wait_source_oui() is called before intel_dp->last_oui_write is
> > > set and this results in a 10min timeout because our jiffies are set to
> > > ULONG_MAX - (10 * 60 * HZ);
> > 
> > After talking with kettenis@ I think the following diff is better.
> > Starting with 0 jiffies should fix this issue.
> > Unless we want to do the linux madness and set it to
> >     ((unsigned long)(unsigned int) (-300*HZ))
> > 
> > -- 
> > :wq Claudio
> > 
> > Index: kern_clock.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/kern_clock.c,v
> > retrieving revision 1.109
> > diff -u -p -r1.109 kern_clock.c
> > --- kern_clock.c    25 Jul 2023 18:16:19 -0000      1.109
> > +++ kern_clock.c    31 Jul 2023 20:01:57 -0000
> > @@ -84,7 +84,7 @@ int       profhz;
> >  int        profprocs;
> >  int        ticks = INT_MAX - (15 * 60 * HZ);
> >  
> > -volatile unsigned long jiffies = ULONG_MAX - (10 * 60 * HZ);
> > +volatile unsigned long jiffies;
> >  
> >  /*
> >   * Initialize clock frequencies and start both clocks running.
> > 
> 
> I think this is backwards.
> 
> Why are we changing the initial value of jiffies (wide) to resolve a
> problem with the initialization of one struct (narrow)?  Changing the
> initial value of jiffies just masks the root cause.
> 
> Isn't the right thing here to initialize the last-write timestamp when
> the struct is allocated?

This is all in code that is regularly synced with linux and so any local
change there is less then ideal. So it is better to alter the way jiffies
is initalized. jiffies is only there for drm so I don't think it is that
wide of a change.
Btw. on linux jiffies is initalized to:
#define INITIAL_JIFFIES ((unsigned long)(unsigned int) (-300*HZ))
and so the behaviour is different on 32 vs 64bit systems (which is the
worst possible default they could choose). So on the more common 64bit
systems the wrap around no longer happens early and bugs are introduced
because of this without anyone noticing.

Somebody could report this upstream since it is a bug in the intel drm
codebase.
-- 
:wq Claudio

Reply via email to