2011-02-11 17:57:47

by KY Srinivasan

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/staging/hv/channel.c b/drivers/staging/hv/channel.c
index ba9afda..6c292e6 100644
--- a/drivers/staging/hv/channel.c
+++ b/drivers/staging/hv/channel.c
@@ -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);

diff --git a/drivers/staging/hv/connection.c b/drivers/staging/hv/connection.c
index b3ac66e..ed0976a 100644
--- a/drivers/staging/hv/connection.c
+++ b/drivers/staging/hv/connection.c
@@ -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);
diff --git a/drivers/staging/hv/hv.c b/drivers/staging/hv/hv.c
index 021acba..419b4d6 100644
--- a/drivers/staging/hv/hv.c
+++ b/drivers/staging/hv/hv.c
@@ -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]);
}
diff --git a/drivers/staging/hv/netvsc.c b/drivers/staging/hv/netvsc.c
index 4319363..a271aa7 100644
--- a/drivers/staging/hv/netvsc.c
+++ b/drivers/staging/hv/netvsc.c
@@ -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;
}

--
1.5.5.6


2011-02-11 18:42:47

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions

On Fri, Feb 11, 2011 at 09:59:00AM -0800, K. Y. Srinivasan wrote:
> --- a/drivers/staging/hv/hv.c
> +++ b/drivers/staging/hv/hv.c
> @@ -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

2011-02-11 21:01:10

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions



> -----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
>
> On Fri, Feb 11, 2011 at 09:59:00AM -0800, K. Y. Srinivasan wrote:
> > --- a/drivers/staging/hv/hv.c
> > +++ b/drivers/staging/hv/hv.c
> > @@ -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?

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
>
> thanks,
>
> greg k-h

2011-02-11 21:26:07

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions

On Fri, Feb 11, 2011 at 08:55:56PM +0000, KY Srinivasan wrote:
>
>
> > -----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
> >
> > On Fri, Feb 11, 2011 at 09:59:00AM -0800, K. Y. Srinivasan wrote:
> > > --- a/drivers/staging/hv/hv.c
> > > +++ b/drivers/staging/hv/hv.c
> > > @@ -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?
>
> 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?

> 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.

> 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

2011-02-11 21:27:16

by Hank Janssen

[permalink] [raw]
Subject: RE: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions



> -----Original Message-----
> And then KY Srinivasan spoke on Friday, February 11, 2011 12:56 PM
> > From: Greg KH [mailto:[email protected]]
> > Sent: Friday, February 11, 2011 1:30 PM
> > > - 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?
>
> 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.

2011-02-11 21:30:51

by Hank Janssen

[permalink] [raw]
Subject: RE: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions



> -----Original Message-----
> From: Greg KH [mailto:[email protected]] On Friday, February 11, 2011 1:24 PM
> On Fri, Feb 11, 2011 at 08:55:56PM +0000, KY Srinivasan wrote:
> > > 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.
>
> 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 :)

>
> > 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.
>
> > 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.

2011-02-11 21:37:28

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/3]: Staging: hv: Use native page allocation/free functions



> -----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
>
> On Fri, Feb 11, 2011 at 08:55:56PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----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
> > >
> > > On Fri, Feb 11, 2011 at 09:59:00AM -0800, K. Y. Srinivasan wrote:
> > > > --- a/drivers/staging/hv/hv.c
> > > > +++ b/drivers/staging/hv/hv.c
> > > > @@ -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?
> >
> > 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?
>
> > 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.
>
> > 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
>
> thanks,
>
> greg k-h