On Mon, Oct 28, 2019 at 05:35:15PM -0600, Jens Axboe wrote: > On 10/28/19 5:30 PM, Kees Cook wrote: > > On Mon, Oct 28, 2019 at 05:27:59PM -0600, Jens Axboe wrote: > >> On 10/28/19 5:05 PM, coverity-bot wrote: > >>> Hello! > >>> > >>> This is an experimental automated report about issues detected by Coverity > >>> from a scan of next-20191025 as part of the linux-next weekly scan project: > >>> https://scan.coverity.com/projects/linux-next-weekly-scan > >>> > >>> You're getting this email because you were associated with the identified > >>> lines of code (noted below) that were touched by recent commits: > >>> > >>> 46134db8fdc5 ("io-wq: small threadpool implementation for io_uring") > >>> > >>> Coverity reported the following: > >>> > >>> *** CID 1487365: Program hangs (LOCK) > >>> /fs/io-wq.c: 349 in io_wqe_worker() > >>> 343 io_worker_handle_work(worker); > >>> 344 else > >>> 345 spin_unlock(&wqe->lock); > >>> 346 } > >>> 347 > >>> 348 io_worker_exit(worker); > >>> vvv CID 1487365: Program hangs (LOCK) > >>> vvv Returning without unlocking "(*wqe).lock". > >>> 349 return 0; > >>> 350 } > >>> 351 > >>> 352 /* > >>> 353 * Check head of free list for an available worker. If one isn't available, > >>> 354 * caller must wake up the wq manager to create one. > >>> > >>> If this is a false positive, please let us know so we can mark it as > >>> such, or teach the Coverity rules to be smarter. If not, please make > >>> sure fixes get into linux-next. :) For patches fixing this, please > >>> include: > >> > >> It's a false positive, lock is dropped on non-zero return. > > > > Does that happen in the caller side? I'll see if I can figure out how to > > teach coverity about that... Hmmm > > I was trying to use the right incantations of __release() etc to shut up > the checker as well. So if there are things I could be improving on that > side, do let me know. > > As mentioned in other emails, the linux-next version is somewhat outdated > at this point. My for-next branch has the latest version. Yeah, I think I see Coverity's confusion: this code is the work queue runner, IIUC, so the locking is pretty special. I'll try to see if there is a way to improve this. -- Kees Cook
