Dan Carpenter <dan.carpenter@xxxxxxxxxx> writes:
> Hello Vitaly Kuznetsov,
>
> The patch b05d8d9ef5ef: "Drivers: hv: hv_balloon: eliminate the
> trylock path in acquire/release_region_mutex" from Feb 28, 2015,
> leads to the following static checker warning:
>
> drivers/hv/hv_balloon.c:669 hv_mem_hot_add()
> warn: 'mutex:&dm_device.ha_region_mutex' is sometimes locked here and sometimes unlocked.
>
Thanks, I think I addressed the issue in my '[PATCH] Drivers: hv:
hv_balloon: keep locks balanced on add_memory() failure' which is still
pending.
> drivers/hv/hv_balloon.c
> 609 static void hv_mem_hot_add(unsigned long start, unsigned long size,
> 610 unsigned long pfn_count,
> 611 struct hv_hotadd_state *has)
> 612 {
> 613 int ret = 0;
> 614 int i, nid;
> 615 unsigned long start_pfn;
> 616 unsigned long processed_pfn;
> 617 unsigned long total_pfn = pfn_count;
> 618
> 619 for (i = 0; i < (size/HA_CHUNK); i++) {
> 620 start_pfn = start + (i * HA_CHUNK);
> 621 has->ha_end_pfn += HA_CHUNK;
> 622
> 623 if (total_pfn > HA_CHUNK) {
> 624 processed_pfn = HA_CHUNK;
> 625 total_pfn -= HA_CHUNK;
> 626 } else {
> 627 processed_pfn = total_pfn;
> 628 total_pfn = 0;
> 629 }
> 630
> 631 has->covered_end_pfn += processed_pfn;
> 632
> 633 init_completion(&dm_device.ol_waitevent);
> 634 dm_device.ha_waiting = true;
> 635
> 636 mutex_unlock(&dm_device.ha_region_mutex);
> 637 nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
> 638 ret = add_memory(nid, PFN_PHYS((start_pfn)),
> 639 (HA_CHUNK << PAGE_SHIFT));
> 640
> 641 if (ret) {
> 642 pr_info("hot_add memory failed error is %d\n", ret);
> 643 if (ret == -EEXIST) {
> 644 /*
> 645 * This error indicates that the error
> 646 * is not a transient failure. This is the
> 647 * case where the guest's physical address map
> 648 * precludes hot adding memory. Stop all further
> 649 * memory hot-add.
> 650 */
> 651 do_hot_add = false;
> 652 }
> 653 has->ha_end_pfn -= HA_CHUNK;
> 654 has->covered_end_pfn -= processed_pfn;
>
> The callers assume we return with the lock held so they double unlock.
> Probably double unlocking is not so terrible as double locking...
>
> 655 break;
> 656 }
> 657
> 658 /*
> 659 * Wait for the memory block to be onlined.
> 660 * Since the hot add has succeeded, it is ok to
> 661 * proceed even if the pages in the hot added region
> 662 * have not been "onlined" within the allowed time.
> 663 */
> 664 wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
> 665 mutex_lock(&dm_device.ha_region_mutex);
> 666 post_status(&dm_device);
> 667 }
> 668
> 669 return;
> 670 }
>
> regards,
> dan carpenter
--
Vitaly
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
