[bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()

[bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()

[Dan Carpenter]

Hello David Hildenbrand,

The patch 6cc26d77613a: "virtio-mem: use page_offline_(start|end)
when setting PageOffline()" from Jun 30, 2021, leads to the following
Smatch static checker warning:

	drivers/virtio/virtio_mem.c:1072 virtio_mem_set_fake_offline()
	warn: sleeping in atomic context

drivers/virtio/virtio_mem.c
    1069 static void virtio_mem_set_fake_offline(unsigned long pfn,
    1070 					unsigned long nr_pages, bool onlined)
    1071 {
--> 1072 	page_offline_begin();

virtio_mem_online_page_cb() is holding rcu_read_lock() so calling
page_offline_begin() here is sleeping in atomic bug.

    1073 	for (; nr_pages--; pfn++) {
    1074 		struct page *page = pfn_to_page(pfn);
    1075 
    1076 		__SetPageOffline(page);
    1077 		if (!onlined) {
    1078 			SetPageDirty(page);
    1079 			/* FIXME: remove after cleanups */
    1080 			ClearPageReserved(page);
    1081 		}
    1082 	}
    1083 	page_offline_end();
    1084 }

regards,
dan carpenter
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()

[David Hildenbrand]

On 25.08.21 11:40, Dan Carpenter wrote:
> Hello David Hildenbrand,
> 
> The patch 6cc26d77613a: "virtio-mem: use page_offline_(start|end)
> when setting PageOffline()" from Jun 30, 2021, leads to the following
> Smatch static checker warning:
> 
> 	drivers/virtio/virtio_mem.c:1072 virtio_mem_set_fake_offline()
> 	warn: sleeping in atomic context
> 
> drivers/virtio/virtio_mem.c
>      1069 static void virtio_mem_set_fake_offline(unsigned long pfn,
>      1070 					unsigned long nr_pages, bool onlined)
>      1071 {
> --> 1072 	page_offline_begin();
> 
> virtio_mem_online_page_cb() is holding rcu_read_lock() so calling
> page_offline_begin() here is sleeping in atomic bug.
> 
>      1073 	for (; nr_pages--; pfn++) {
>      1074 		struct page *page = pfn_to_page(pfn);
>      1075
>      1076 		__SetPageOffline(page);
>      1077 		if (!onlined) {
>      1078 			SetPageDirty(page);
>      1079 			/* FIXME: remove after cleanups */
>      1080 			ClearPageReserved(page);
>      1081 		}
>      1082 	}
>      1083 	page_offline_end();
>      1084 }

Ah, thanks for that report!


diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 09ed55de07d7..9e9922c60da4 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1242,12 +1242,19 @@ static void virtio_mem_online_page_cb(struct page *page, unsigned int order)
                         do_online = virtio_mem_bbm_get_bb_state(vm, id) !=
                                     VIRTIO_MEM_BBM_BB_FAKE_OFFLINE;
                 }
+
+               /*
+                * virtio_mem_set_fake_offline() might sleep, we don't need
+                * the device anymore. See virtio_mem_remove() how races
+                * with device removal are avoided.
+                */
+               rcu_read_unlock();
+
                 if (do_online)
                         generic_online_page(page, order);
                 else
                         virtio_mem_set_fake_offline(PFN_DOWN(addr), 1 << order,
                                                     false);
-               rcu_read_unlock();
                 return;
         }
         rcu_read_unlock();


Will send out a patch shortly.

-- 
Thanks,

David / dhildenb

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization