Skip to content

Commit 3da7326

Browse files
lorelei-sakaiMikulas Patocka
authored andcommitted
dm vdo indexer: don't read request structure after enqueuing
The function get_volume_page_protected may place a request on a queue for another thread to process asynchronously. When this happens, the volume should not read the request from the original thread. This can not currently cause problems, due to the way request processing is handled, but it is not safe in general. Reviewed-by: Ken Raeburn <raeburn@redhat.com> Signed-off-by: Matthew Sakai <msakai@redhat.com> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
1 parent e939127 commit 3da7326

File tree

1 file changed

+13
-11
lines changed

1 file changed

+13
-11
lines changed

drivers/md/dm-vdo/indexer/volume.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -754,10 +754,11 @@ static int get_volume_page_protected(struct volume *volume, struct uds_request *
754754
u32 physical_page, struct cached_page **page_ptr)
755755
{
756756
struct cached_page *page;
757+
unsigned int zone_number = request->zone_number;
757758

758759
get_page_from_cache(&volume->page_cache, physical_page, &page);
759760
if (page != NULL) {
760-
if (request->zone_number == 0) {
761+
if (zone_number == 0) {
761762
/* Only one zone is allowed to update the LRU. */
762763
make_page_most_recent(&volume->page_cache, page);
763764
}
@@ -767,7 +768,7 @@ static int get_volume_page_protected(struct volume *volume, struct uds_request *
767768
}
768769

769770
/* Prepare to enqueue a read for the page. */
770-
end_pending_search(&volume->page_cache, request->zone_number);
771+
end_pending_search(&volume->page_cache, zone_number);
771772
mutex_lock(&volume->read_threads_mutex);
772773

773774
/*
@@ -787,8 +788,7 @@ static int get_volume_page_protected(struct volume *volume, struct uds_request *
787788
* the order does not matter for correctness as it does below.
788789
*/
789790
mutex_unlock(&volume->read_threads_mutex);
790-
begin_pending_search(&volume->page_cache, physical_page,
791-
request->zone_number);
791+
begin_pending_search(&volume->page_cache, physical_page, zone_number);
792792
return UDS_QUEUED;
793793
}
794794

@@ -797,7 +797,7 @@ static int get_volume_page_protected(struct volume *volume, struct uds_request *
797797
* "search pending" state in careful order so no other thread can mess with the data before
798798
* the caller gets to look at it.
799799
*/
800-
begin_pending_search(&volume->page_cache, physical_page, request->zone_number);
800+
begin_pending_search(&volume->page_cache, physical_page, zone_number);
801801
mutex_unlock(&volume->read_threads_mutex);
802802
*page_ptr = page;
803803
return UDS_SUCCESS;
@@ -849,6 +849,7 @@ static int search_cached_index_page(struct volume *volume, struct uds_request *r
849849
{
850850
int result;
851851
struct cached_page *page = NULL;
852+
unsigned int zone_number = request->zone_number;
852853
u32 physical_page = map_to_physical_page(volume->geometry, chapter,
853854
index_page_number);
854855

@@ -858,18 +859,18 @@ static int search_cached_index_page(struct volume *volume, struct uds_request *r
858859
* invalidation by the reader thread, before the reader thread has noticed that the
859860
* invalidate_counter has been incremented.
860861
*/
861-
begin_pending_search(&volume->page_cache, physical_page, request->zone_number);
862+
begin_pending_search(&volume->page_cache, physical_page, zone_number);
862863

863864
result = get_volume_page_protected(volume, request, physical_page, &page);
864865
if (result != UDS_SUCCESS) {
865-
end_pending_search(&volume->page_cache, request->zone_number);
866+
end_pending_search(&volume->page_cache, zone_number);
866867
return result;
867868
}
868869

869870
result = uds_search_chapter_index_page(&page->index_page, volume->geometry,
870871
&request->record_name,
871872
record_page_number);
872-
end_pending_search(&volume->page_cache, request->zone_number);
873+
end_pending_search(&volume->page_cache, zone_number);
873874
return result;
874875
}
875876

@@ -882,6 +883,7 @@ int uds_search_cached_record_page(struct volume *volume, struct uds_request *req
882883
{
883884
struct cached_page *record_page;
884885
struct index_geometry *geometry = volume->geometry;
886+
unsigned int zone_number = request->zone_number;
885887
int result;
886888
u32 physical_page, page_number;
887889

@@ -905,19 +907,19 @@ int uds_search_cached_record_page(struct volume *volume, struct uds_request *req
905907
* invalidation by the reader thread, before the reader thread has noticed that the
906908
* invalidate_counter has been incremented.
907909
*/
908-
begin_pending_search(&volume->page_cache, physical_page, request->zone_number);
910+
begin_pending_search(&volume->page_cache, physical_page, zone_number);
909911

910912
result = get_volume_page_protected(volume, request, physical_page, &record_page);
911913
if (result != UDS_SUCCESS) {
912-
end_pending_search(&volume->page_cache, request->zone_number);
914+
end_pending_search(&volume->page_cache, zone_number);
913915
return result;
914916
}
915917

916918
if (search_record_page(dm_bufio_get_block_data(record_page->buffer),
917919
&request->record_name, geometry, &request->old_metadata))
918920
*found = true;
919921

920-
end_pending_search(&volume->page_cache, request->zone_number);
922+
end_pending_search(&volume->page_cache, zone_number);
921923
return UDS_SUCCESS;
922924
}
923925

0 commit comments

Comments
 (0)