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

Reply via email to