On Fri, Feb 09 2024 at 8:06P -0500, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > Hello Matthew Sakai, > > The patch e08b7fe0c6fa: "dm vdo: implement the chapter volume store" > from Nov 16, 2023 (linux-next), leads to the following Smatch static > checker warning: > > drivers/md/dm-vdo/volume.c:597 process_entry() warn: inconsistent returns '&volume->read_threads_mutex'. > Locked on : 552,580,588,597 > Unlocked on: 565 > > drivers/md/dm-vdo/volume.c > 542 static int process_entry(struct volume *volume, struct queued_read *entry) > 543 { > 544 u32 page_number = entry->physical_page; > 545 struct uds_request *request; > 546 struct cached_page *page = NULL; > 547 u8 *page_data; > 548 int result; > 549 > 550 if (entry->invalid) { > 551 uds_log_debug("Requeuing requests for invalid page"); > 552 return UDS_SUCCESS; > 553 } > 554 > 555 page = select_victim_in_cache(&volume->page_cache); > 556 > 557 uds_unlock_mutex(&volume->read_threads_mutex); > 558 page_data = dm_bufio_read(volume->client, page_number, &page->buffer); > 559 if (IS_ERR(page_data)) { > 560 result = -PTR_ERR(page_data); > 561 uds_log_warning_strerror(result, > 562 "error reading physical page %u from volume", > 563 page_number); > 564 cancel_page_in_cache(&volume->page_cache, page_number, page); > 565 return result; > > This is the only return where we aren't holding > uds_lock_mutex(&volume->read_threads_mutex). It looks like this > will lead to a double unlock in the caller. I've fixed this and added your Reported-by Thanks, Mike > > 566 } > 567 uds_lock_mutex(&volume->read_threads_mutex); > 568 > 569 if (entry->invalid) { > 570 uds_log_warning("Page %u invalidated after read", page_number); > 571 cancel_page_in_cache(&volume->page_cache, page_number, page); > 572 return UDS_SUCCESS; > 573 } > 574 > 575 if (!is_record_page(volume->geometry, page_number)) { > 576 result = initialize_index_page(volume, page_number, page); > 577 if (result != UDS_SUCCESS) { > 578 uds_log_warning("Error initializing chapter index page"); > 579 cancel_page_in_cache(&volume->page_cache, page_number, page); > 580 return result; > 581 } > 582 } > 583 > 584 result = put_page_in_cache(&volume->page_cache, page_number, page); > 585 if (result != UDS_SUCCESS) { > 586 uds_log_warning("Error putting page %u in cache", page_number); > 587 cancel_page_in_cache(&volume->page_cache, page_number, page); > 588 return result; > 589 } > 590 > 591 request = entry->first_request; > 592 while ((request != NULL) && (result == UDS_SUCCESS)) { > 593 result = search_page(page, volume, request, page_number); > 594 request = request->next_request; > 595 } > 596 > --> 597 return result; > 598 } > > regards, > dan carpenter >