Skip to contentSkip to conversation

: Staging: hv: Use native page allocation/free functions

PATCH In review K. Y. Srinivasan 3 participants 7 messages Feb 11, 2011

Subsystems (2)
Staging virtio core
lore
Share & tools
lore

K. Y. Srinivasan

author
#0 lore raw

PATCH 1/3 : Staging: hv: Use native page allocation/free functions

In preperation for getting rid of the osd.[ch] files; change all page allocation/free functions to use native interfaces.

Signed-off-by K. Y. Srinivasan <[email protected]>
Signed-off-by Hank Janssen <[email protected]>
---
 drivers/staging/hv/channel.c    |   12 +++++++-----
 drivers/staging/hv/connection.c |   13 ++++++++-----
 drivers/staging/hv/hv.c         |   15 ++++++++++-----
 drivers/staging/hv/netvsc.c     |   14 ++++++++------
 4 files changed, 33 insertions(+), 21 deletions(-)
drivers/staging/hv/channel.c +7 -5
index ba9afda..6c292e6 100644
@@ -180,8 +180,9 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 	newchannel->channel_callback_context = context;
 
 	/* Allocate the ring buffer */
-	out = osd_page_alloc((send_ringbuffer_size + recv_ringbuffer_size)
-			     >> PAGE_SHIFT);
+	out = (void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
+		get_order(send_ringbuffer_size + recv_ringbuffer_size));
+
 	if (!out)
 		return -ENOMEM;
 

@@ -300,8 +301,8 @@ Cleanup:
 errorout:
 	ringbuffer_cleanup(&newchannel->outbound);
 	ringbuffer_cleanup(&newchannel->inbound);
-	osd_page_free(out, (send_ringbuffer_size + recv_ringbuffer_size)
-		     >> PAGE_SHIFT);
+	free_pages((unsigned long)out,
+		get_order(send_ringbuffer_size + recv_ringbuffer_size));
 	kfree(openInfo);
 	return err;
 }

@@ -686,7 +687,8 @@ void vmbus_close(struct vmbus_channel *channel)
 	ringbuffer_cleanup(&channel->outbound);
 	ringbuffer_cleanup(&channel->inbound);
 
-	osd_page_free(channel->ringbuffer_pages, channel->ringbuffer_pagecount);
+	free_pages((unsigned long)channel->ringbuffer_pages,
+		get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
 
 	kfree(info);
drivers/staging/hv/connection.c +8 -5
index b3ac66e..ed0976a 100644
@@ -66,7 +66,8 @@ int vmbus_connect(void)
 	 * Setup the vmbus event connection for channel interrupt
 	 * abstraction stuff
 	 */
-	vmbus_connection.int_page = osd_page_alloc(1);
+	vmbus_connection.int_page =
+	(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO, 0);
 	if (vmbus_connection.int_page == NULL) {
 		ret = -1;
 		goto Cleanup;

@@ -81,7 +82,8 @@ int vmbus_connect(void)
 	 * Setup the monitor notification facility. The 1st page for
 	 * parent->child and the 2nd page for child->parent
 	 */
-	vmbus_connection.monitor_pages = osd_page_alloc(2);
+	vmbus_connection.monitor_pages =
+	(void *)__get_free_pages((GFP_KERNEL|__GFP_ZERO), 1);
 	if (vmbus_connection.monitor_pages == NULL) {
 		ret = -1;
 		goto Cleanup;

@@ -162,12 +164,12 @@ Cleanup:
 		destroy_workqueue(vmbus_connection.work_queue);
 
 	if (vmbus_connection.int_page) {
-		osd_page_free(vmbus_connection.int_page, 1);
+		free_pages((unsigned long)vmbus_connection.int_page, 0);
 		vmbus_connection.int_page = NULL;
 	}
 
 	if (vmbus_connection.monitor_pages) {
-		osd_page_free(vmbus_connection.monitor_pages, 2);
+		free_pages((unsigned long)vmbus_connection.monitor_pages, 1);
 		vmbus_connection.monitor_pages = NULL;
 	}
 

@@ -203,7 +205,8 @@ int vmbus_disconnect(void)
 	if (ret != 0)
 		goto Cleanup;
 
-	osd_page_free(vmbus_connection.int_page, 1);
+	free_pages((unsigned long)vmbus_connection.int_page, 0);
+	free_pages((unsigned long)vmbus_connection.monitor_pages, 1);
 
 	/* TODO: iterate thru the msg list and free up */
 	destroy_workqueue(vmbus_connection.work_queue);
drivers/staging/hv/hv.c +10 -5
index 021acba..419b4d6 100644
@@ -230,7 +230,12 @@ int hv_init(void)
 	* Allocate the hypercall page memory
 	* virtaddr = osd_page_alloc(1);
 	*/
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
+#else
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX)));
+#endif
 
 	if (!virtaddr) {
 		DPRINT_ERR(VMBUS,

@@ -462,10 +467,10 @@ void hv_synic_init(void *irqarg)
 
 Cleanup:
 	if (hv_context.synic_event_page[cpu])
-		osd_page_free(hv_context.synic_event_page[cpu], 1);
+		free_page((unsigned long)hv_context.synic_event_page[cpu]);
 
 	if (hv_context.synic_message_page[cpu])
-		osd_page_free(hv_context.synic_message_page[cpu], 1);
+		free_page((unsigned long)hv_context.synic_message_page[cpu]);
 	return;
 }
 

@@ -502,6 +507,6 @@ void hv_synic_cleanup(void *arg)
 
 	wrmsrl(HV_X64_MSR_SIEFP, siefp.as_uint64);
 
-	osd_page_free(hv_context.synic_message_page[cpu], 1);
-	osd_page_free(hv_context.synic_event_page[cpu], 1);
+	free_page((unsigned long)hv_context.synic_message_page[cpu]);
+	free_page((unsigned long)hv_context.synic_event_page[cpu]);
 }
drivers/staging/hv/netvsc.c +8 -6
index 4319363..a271aa7 100644
@@ -223,7 +223,8 @@ static int netvsc_init_recv_buf(struct hv_device *device)
 	/* ASSERT((netDevice->ReceiveBufferSize & (PAGE_SIZE - 1)) == 0); */
 
 	net_device->recv_buf =
-		osd_page_alloc(net_device->recv_buf_size >> PAGE_SHIFT);
+		(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
+				get_order(net_device->recv_buf_size));
 	if (!net_device->recv_buf) {
 		DPRINT_ERR(NETVSC,
 			   "unable to allocate receive buffer of size %d",

@@ -360,7 +361,8 @@ static int netvsc_init_send_buf(struct hv_device *device)
 	/* ASSERT((netDevice->SendBufferSize & (PAGE_SIZE - 1)) == 0); */
 
 	net_device->send_buf =
-		osd_page_alloc(net_device->send_buf_size >> PAGE_SHIFT);
+		(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
+				get_order(net_device->send_buf_size));
 	if (!net_device->send_buf) {
 		DPRINT_ERR(NETVSC, "unable to allocate send buffer of size %d",
 			   net_device->send_buf_size);

@@ -498,8 +500,8 @@ static int netvsc_destroy_recv_buf(struct netvsc_device *net_device)
 		DPRINT_INFO(NETVSC, "Freeing up receive buffer...");
 
 		/* Free up the receive buffer */
-		osd_page_free(net_device->recv_buf,
-			     net_device->recv_buf_size >> PAGE_SHIFT);
+		free_pages((unsigned long)net_device->recv_buf,
+			get_order(net_device->recv_buf_size));
 		net_device->recv_buf = NULL;
 	}
 

@@ -574,8 +576,8 @@ static int netvsc_destroy_send_buf(struct netvsc_device *net_device)
 		DPRINT_INFO(NETVSC, "Freeing up send buffer...");
 
 		/* Free up the receive buffer */
-		osd_page_free(net_device->send_buf,
-			     net_device->send_buf_size >> PAGE_SHIFT);
+		free_pages((unsigned long)net_device->send_buf,
+				get_order(net_device->send_buf_size));
 		net_device->send_buf = NULL;
 	}
Signature · 1 line
1.5.5.6
#1 lore raw

drivers/staging/hv/hv.c int hv_init(void) · line 238

Quoted 13 lines · K. Y. Srinivasan
drivers/staging/hv/hv.c +6 -1
@@ -230,7 +230,12 @@ int hv_init(void)
 	* Allocate the hypercall page memory
 	* virtaddr = osd_page_alloc(1);
 	*/
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);
+#else
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX)));
+#endif

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this necessary, it should be ok to do the same type of allocation no matter what the processor is, right?

thanks,

greg k-h

author
#2 lore raw
Quoted 21 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Friday, February 11, 2011 1:30 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; Hank Janssen
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions

drivers/staging/hv/hv.c int hv_init(void) · line 234

Quoted 9 lines · K. Y. Srinivasan
drivers/staging/hv/hv.c +2 -1
@@ -230,7 +230,12 @@ int hv_init(void)
 	* Allocate the hypercall page memory
 	* virtaddr = osd_page_alloc(1);
 	*/
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);

#else

Quoted 7 lines

drivers/staging/hv/hv.c int hv_init(void) · line 236

Quoted 2 lines
drivers/staging/hv/hv.c +2 -0
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX))); #endif

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this necessary, it should be ok to do the same type of allocation no matter what the processor is, right?

You are right Greg; I don't think there is a need to specify different page protection bits based on the architecture - PAGE_KERNEL_EXEC should be enough. However, this is the code that is currently in the tree - refer to osd.c. If it is ok with you, I could submit an additional patch to clean this up.

Regards,

K. Y

Quoted 4 lines

thanks,

greg k-h

#3 lore raw
Quoted 29 lines · KY Srinivasan
Quoted 21 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Friday, February 11, 2011 1:30 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; Hank Janssen
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions

drivers/staging/hv/hv.c int hv_init(void) · line 234

Quoted 9 lines · K. Y. Srinivasan
drivers/staging/hv/hv.c +2 -1
@@ -230,7 +230,12 @@ int hv_init(void)
 	* Allocate the hypercall page memory
 	* virtaddr = osd_page_alloc(1);
 	*/
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);

#else

Quoted 7 lines

drivers/staging/hv/hv.c int hv_init(void) · line 236

Quoted 2 lines
drivers/staging/hv/hv.c +2 -0
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX))); #endif

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this necessary, it should be ok to do the same type of allocation no matter what the processor is, right?

Quoted 2 lines

You are right Greg; I don't think there is a need to specify different page protection bits based on the architecture - PAGE_KERNEL_EXEC should be enough.

I thought so, but for some reason Hank said there this was needed. Hank, is it still true?

Quoted 1 line

However, this is the code that is currently in the tree - refer to osd.c.

Oh, I remember, it's not a critique of this patch, it just reminded me of this question I always had for this code.

Quoted 1 line

If it is ok with you, I could submit an additional patch to clean this up.

If Hank says it is ok, and you all test it to verify nothing breaks, please send it on.

thanks,

greg k-h

#4 lore raw
Quoted 10 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]] On Friday, February 11, 2011 1:24 PM

Quoted 5 lines · KY Srinivasan
Quoted 4 lines

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this necessary, it should be ok to do the same type of allocation no matter what the processor is, right?

Quoted 6 lines
Quoted 2 lines

You are right Greg; I don't think there is a need to specify different page protection bits based on the architecture - PAGE_KERNEL_EXEC

should be enough.

I thought so, but for some reason Hank said there this was needed. Hank, is it still true?

I recall we did it for older versions of Linux but I do not recall why. Something from way before 2.6.18, the reason of which I seem to have Purged due to age :)

Quoted 10 lines
Quoted 2 lines

However, this is the code that is currently in the tree - refer to osd.c.

Oh, I remember, it's not a critique of this patch, it just reminded me of this question I always had for this code.

Quoted 1 line

If it is ok with you, I could submit an additional patch to clean this up.

If Hank says it is ok, and you all test it to verify nothing breaks, please send it on.

If you could accept the patch as is and I will work with Ky to see if nothing breaks If we change this part to what you are suggesting? If nothing breaks we will submit A followup patch to remove those lines.

Hank.

author
#5 lore raw
In reply to Greg KH
Quoted 52 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Friday, February 11, 2011 4:24 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; Hank Janssen
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions

Quoted 9 lines · KY Srinivasan
Quoted 9 lines

-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Friday, February 11, 2011 1:30 PM
To: KY Srinivasan
Cc: [email protected]; [email protected]; [email protected]; Hank Janssen
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free

functions

Quoted 12 lines
Quoted 12 lines · K. Y. Srinivasan

drivers/staging/hv/hv.c int hv_init(void) · line 234

Quoted 9 lines
drivers/staging/hv/hv.c +2 -1
@@ -230,7 +230,12 @@ int hv_init(void)
 	* Allocate the hypercall page memory
 	* virtaddr = osd_page_alloc(1);
 	*/
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_EXEC);

#else

Quoted 5 lines
Quoted 5 lines

drivers/staging/hv/hv.c int hv_init(void) · line 236

Quoted 2 lines
drivers/staging/hv/hv.c +2 -0
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX))); #endif

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this

necessary, it

Quoted 1 line
Quoted 1 line

should be ok to do the same type of allocation no matter what the processor

is,

Quoted 4 lines
Quoted 1 line

right?

You are right Greg; I don't think there is a need to specify different page protection bits based on the architecture - PAGE_KERNEL_EXEC should be

enough.

I thought so, but for some reason Hank said there this was needed. Hank, is it still true?

Quoted 1 line

However, this is the code that is currently in the tree - refer to osd.c.

Quoted 7 lines

Oh, I remember, it's not a critique of this patch, it just reminded me of this question I always had for this code.

Quoted 1 line

If it is ok with you, I could submit an additional patch to clean this up.

If Hank says it is ok, and you all test it to verify nothing breaks, please send it on.

Will do; the new patch will be on top of the 3 patch sequence I sent earlier today.

Regards,

K. Y

Quoted 4 lines

thanks,

greg k-h

#6 lore raw
In reply to KY Srinivasan
Quoted 19 lines

-----Original Message-----
And then KY Srinivasan spoke on Friday, February 11, 2011 12:56 PM

Quoted 5 lines

From: Greg KH [mailto:[email protected]]
Sent: Friday, February 11, 2011 1:30 PM

drivers/staging/hv/hv.c int hv_init(void) · line 236

Quoted 3 lines
drivers/staging/hv/hv.c +2 -1
-	virtaddr = osd_virtual_alloc_exec(PAGE_SIZE);
+#ifdef __x86_64__
+	virtaddr = __vmalloc(PAGE_SIZE, GFP_KERNEL,

PAGE_KERNEL_EXEC);

Quoted 8 lines

#else

Review

Quoted 2 lines
(quoted diff) +2 -0
+	virtaddr =  __vmalloc(PAGE_SIZE, GFP_KERNEL,
+			__pgprot(__PAGE_KERNEL & (~_PAGE_NX))); #endif

I'm not saying this patch is wrong at all, but I still don't understand why this is different depending on the architecture of the machine. Why is this necessary, it should be ok to do the same type of allocation no matter what the processor is, right?

Quoted 6 lines

You are right Greg; I don't think there is a need to specify different page protection bits based on the architecture - PAGE_KERNEL_EXEC should be enough.
However, this is the code that is currently in the tree - refer to osd.c. If it is ok with you, I could submit an additional patch to clean this up.

I seem to recall that we did it for very old versions of Linux (pre 2.6.18) But I cannot for the life of me remember why.

Hank.