NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/56988 (Bluetooth stack initializes bt_lock too late)



The following reply was made to PR kern/56988; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Iain Hibbert <plunky%ogmig.net@localhost>
Cc: gnats-bugs%netbsd.org@localhost
Subject: Re: kern/56988 (Bluetooth stack initializes bt_lock too late)
Date: Fri, 6 Jan 2023 22:19:47 +0000

 > Date: Fri, 6 Jan 2023 20:58:12 +0000 (GMT)
 > From: Iain Hibbert <plunky%ogmig.net@localhost>
 > 
 > Ok, and I note that RUN_ONCE is a little racy as it doesn't hold 
 > another caller back, and this function should be quick but it does 
 > alloc(PR_WAITOK)
 
 RUN_ONCE isn't racy -- there is an underlying lock and condvar, which
 are initialized very early in main.  But it scatters large-scale
 subsystem dependencies deep inside the logic of the code, instead of
 at a higher level like the module declarations.
 
 It may be nice for modules to be loadable and unloadable, but we can
 also take advantage of the concept for organizing monolithic kernels
 even if the components can't be loaded or unloaded.
 
 > > 1. Pull up the RUN_ONCE approach to netbsd-10.
 > > 
 > >    Test: Insert kpause(10*hz) in bt_init_once, verify a machine with
 > >    ubt(4) boots without crashing on the mutex.
 > 
 > I can do that, but I'm not sure its useful? Thinking it through if kpause 
 > was before the alloc the second caller would crash taking the mutex, and 
 > if after the alloc it would never crash just delay the boot whichever got 
 > there first.
 
 That's exactly the idea -- do it before initializing bt_lock.
 
 - If this approach works, it will prevent the race -- anyone trying to
   use bt_lock will wait until the first RUN_ONCE call has completed.
 
 - If this approach doesn't work, it will likely crash again -- there's
   still a path to using bt_lock that doesn't wait for the RUN_ONCE.
 
 We don't keep the kpause(10*hz) committed, of course -- just use it to
 test, and then commit without the kpause.
 


Home | Main Index | Thread Index | Old Index