Dan Carpenter
2021-Aug-25 09:40 UTC
[bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()
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
David Hildenbrand
2021-Aug-25 09:56 UTC
[bug report] virtio-mem: use page_offline_(start|end) when setting PageOffline()
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