syz-ci: move build semaphore closer to builds

Currently it's held during the whole job processing, which can take too
long.

Adjust it so that it's only taken when we really begin to build the
kernel or syzkaller.

4 files changed, 19 insertions, 24 deletions

diff --git a/syz-ci/jobs.go b/syz-ci/jobs.go
index ed999a5ff..7f19a862f 100644
--- a/syz-ci/jobs.go
+++ b/syz-ci/jobs.go
@@ -79,7 +79,7 @@ loop:
 		}
 		select {
 		case <-jobTicker.C:
-			if len(kernelBuildSem) != 0 {
+			if buildSem.Available() == 0 {
 				// If normal kernel build is in progress (usually on start), don't query jobs.
 				// Otherwise we claim a job, but can't start it for a while.
 				continue loop
@@ -250,13 +250,6 @@ func (jp *JobProcessor) pollJobs() {
 }
 
 func (jp *JobProcessor) processJob(job *Job) {
-	select {
-	case kernelBuildSem <- struct{}{}:
-	case <-jp.stop:
-		return
-	}
-	defer func() { <-kernelBuildSem }()
-
 	req := job.req
 	log.Logf(0, "starting job %v type %v for manager %v on %v/%v",
 		req.ID, req.Type, req.Manager, req.KernelRepo, req.KernelBranch)
@@ -439,7 +432,8 @@ func (jp *JobProcessor) bisect(job *Job, mgrcfg *mgrconfig.Config) error {
 			Syz:  req.ReproSyz,
 			C:    req.ReproC,
 		},
-		Manager: mgrcfg,
+		Manager:        mgrcfg,
+		BuildSemaphore: buildSem,
 	}
 
 	res, err := bisect.Run(cfg)
@@ -499,7 +493,7 @@ func (jp *JobProcessor) bisect(job *Job, mgrcfg *mgrconfig.Config) error {
 
 func (jp *JobProcessor) testPatch(job *Job, mgrcfg *mgrconfig.Config) error {
 	req, resp, mgr := job.req, job.resp, job.mgr
-	env, err := instance.NewEnv(mgrcfg)
+	env, err := instance.NewEnv(mgrcfg, buildSem)
 	if err != nil {
 		return err
 	}
diff --git a/syz-ci/manager.go b/syz-ci/manager.go
index 161c20171..3744944a5 100644
--- a/syz-ci/manager.go
+++ b/syz-ci/manager.go
@@ -147,10 +147,10 @@ func createManager(cfg *Config, mgrcfg *ManagerConfig, stop chan struct{},
 	return mgr, nil
 }
 
-// Gates kernel builds.
+// Gates kernel builds, syzkaller builds and coverage report generation.
 // Kernel builds take whole machine, so we don't run more than one at a time.
 // Also current image build script uses some global resources (/dev/nbd0) and can't run in parallel.
-var kernelBuildSem = make(chan struct{}, 1)
+var buildSem = instance.NewSemaphore(1)
 
 func (mgr *Manager) loop() {
 	lastCommit := ""
@@ -235,7 +235,7 @@ func (mgr *Manager) pollAndBuild(lastCommit string, latestInfo *BuildInfo) (
 				mgr.configTag != latestInfo.KernelConfigTag) {
 			lastCommit = commit.Hash
 			select {
-			case kernelBuildSem <- struct{}{}:
+			case <-buildSem.WaitC():
 				log.Logf(0, "%v: building kernel...", mgr.name)
 				if err := mgr.build(commit); err != nil {
 					log.Logf(0, "%v: %v", mgr.name, err)
@@ -247,7 +247,7 @@ func (mgr *Manager) pollAndBuild(lastCommit string, latestInfo *BuildInfo) (
 						mgr.Errorf("failed to read build info after build")
 					}
 				}
-				<-kernelBuildSem
+				buildSem.Signal()
 			case <-mgr.stop:
 			}
 		}
@@ -413,7 +413,7 @@ func (mgr *Manager) testImage(imageDir string, info *BuildInfo) error {
 	if !vm.AllowsOvercommit(mgrcfg.Type) {
 		return nil // No support for creating machines out of thin air.
 	}
-	env, err := instance.NewEnv(mgrcfg)
+	env, err := instance.NewEnv(mgrcfg, buildSem)
 	if err != nil {
 		return err
 	}
@@ -763,11 +763,11 @@ func (mgr *Manager) uploadCoverReport() error {
 	}
 	// Report generation can consume lots of memory. Generate one at a time.
 	select {
-	case kernelBuildSem <- struct{}{}:
+	case <-buildSem.WaitC():
 	case <-mgr.stop:
 		return nil
 	}
-	defer func() { <-kernelBuildSem }()
+	defer buildSem.Signal()
 
 	// Get coverage report from manager.
 	addr := mgr.managercfg.HTTP
diff --git a/syz-ci/syz-ci.go b/syz-ci/syz-ci.go
index 790a29f86..f877cfbfc 100644
--- a/syz-ci/syz-ci.go
+++ b/syz-ci/syz-ci.go
@@ -219,7 +219,7 @@ func main() {
 		case <-shutdownPending:
 		case <-updatePending:
 		}
-		kernelBuildSem <- struct{}{} // wait for all current builds
+		buildSem.WaitAll() // wait for all current builds
 		close(stop)
 		wg.Done()
 	}()
diff --git a/syz-ci/updater.go b/syz-ci/updater.go
index 37519f45a..c20dd2f92 100644
--- a/syz-ci/updater.go
+++ b/syz-ci/updater.go
@@ -28,8 +28,9 @@ const (
 
 // SyzUpdater handles everything related to syzkaller updates.
 // As kernel builder, it maintains 2 builds:
-//  - latest: latest known good syzkaller build
-//  - current: currently used syzkaller build
+//   - latest: latest known good syzkaller build
+//   - current: currently used syzkaller build
+//
 // Additionally it updates and restarts the current executable as necessary.
 // Current executable is always built on the same revision as the rest of syzkaller binaries.
 type SyzUpdater struct {
@@ -109,8 +110,8 @@ func NewSyzUpdater(cfg *Config) *SyzUpdater {
 }
 
 // UpdateOnStart does 3 things:
-//  - ensures that the current executable is fresh
-//  - ensures that we have a working syzkaller build in current
+//   - ensures that the current executable is fresh
+//   - ensures that we have a working syzkaller build in current
 func (upd *SyzUpdater) UpdateOnStart(autoupdate bool, shutdown chan struct{}) {
 	os.RemoveAll(upd.currentDir)
 	latestTag := upd.checkLatest()
@@ -221,8 +222,8 @@ func (upd *SyzUpdater) build(commit *vcs.Commit) error {
 	// syzkaller testing may be slowed down by concurrent kernel builds too much
 	// and cause timeout failures, so we serialize it with other builds:
 	// https://groups.google.com/forum/#!msg/syzkaller-openbsd-bugs/o-G3vEsyQp4/f_nFpoNKBQAJ
-	kernelBuildSem <- struct{}{}
-	defer func() { <-kernelBuildSem }()
+	buildSem.Wait()
+	defer buildSem.Signal()
 
 	if upd.descriptions != "" {
 		files, err := ioutil.ReadDir(upd.descriptions)