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.
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
