[PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Marek Marczykowski]

On linux it will end up in munmap anyway, but do not assume any
particular xc_gntshr_munmap implementation details.

Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
---
 tools/libvchan/io.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
index 5ec5fb9..3040099 100644
--- a/tools/libvchan/io.c
+++ b/tools/libvchan/io.c
@@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
 	if (!ctrl)
 		return;
 	if (ctrl->read.order >= PAGE_SHIFT)
-		munmap(ctrl->read.buffer, 1 << ctrl->read.order);
+		xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
+				1 << (ctrl->read.order - PAGE_SHIFT));
 	if (ctrl->write.order >= PAGE_SHIFT)
-		munmap(ctrl->write.buffer, 1 << ctrl->write.order);
+		xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
+				1 << (ctrl->write.order - PAGE_SHIFT));
 	if (ctrl->ring) {
 		if (ctrl->is_server) {
 			ctrl->ring->srv_live = 0;
-- 
1.8.1.4

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[George Dunlap]

On Sat, May 4, 2013 at 11:10 PM, Marek Marczykowski
<marmarek@invisiblethingslab.com> wrote:
> On linux it will end up in munmap anyway, but do not assume any
> particular xc_gntshr_munmap implementation details.
>
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>

Hmm, we don't seem to have a maintainer specifically for this code.

Is this fixing an actual bug for you?  The commit message doesn't
mention anything.  If it's just code clean-up, it should probably wait
until the 4.4 development window opens.

 -George

> ---
>  tools/libvchan/io.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> index 5ec5fb9..3040099 100644
> --- a/tools/libvchan/io.c
> +++ b/tools/libvchan/io.c
> @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
>         if (!ctrl)
>                 return;
>         if (ctrl->read.order >= PAGE_SHIFT)
> -               munmap(ctrl->read.buffer, 1 << ctrl->read.order);
> +               xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
> +                               1 << (ctrl->read.order - PAGE_SHIFT));
>         if (ctrl->write.order >= PAGE_SHIFT)
> -               munmap(ctrl->write.buffer, 1 << ctrl->write.order);
> +               xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
> +                               1 << (ctrl->write.order - PAGE_SHIFT));
>         if (ctrl->ring) {
>                 if (ctrl->is_server) {
>                         ctrl->ring->srv_live = 0;
> --
> 1.8.1.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Wed, 2013-05-08 at 11:48 +0100, George Dunlap wrote:
> On Sat, May 4, 2013 at 11:10 PM, Marek Marczykowski
> <marmarek@invisiblethingslab.com> wrote:
> > On linux it will end up in munmap anyway, but do not assume any
> > particular xc_gntshr_munmap implementation details.
> >
> > Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> 
> Hmm, we don't seem to have a maintainer specifically for this code.

Daniel (already CCd) is the de-facto maintainer. I'd be happy for him to
propose himself as a maintainer.

> 
> Is this fixing an actual bug for you?  The commit message doesn't
> mention anything.  If it's just code clean-up, it should probably wait
> until the 4.4 development window opens.
> 
>  -George
> 
> > ---
> >  tools/libvchan/io.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> > index 5ec5fb9..3040099 100644
> > --- a/tools/libvchan/io.c
> > +++ b/tools/libvchan/io.c
> > @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
> >         if (!ctrl)
> >                 return;
> >         if (ctrl->read.order >= PAGE_SHIFT)
> > -               munmap(ctrl->read.buffer, 1 << ctrl->read.order);
> > +               xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
> > +                               1 << (ctrl->read.order - PAGE_SHIFT));
> >         if (ctrl->write.order >= PAGE_SHIFT)
> > -               munmap(ctrl->write.buffer, 1 << ctrl->write.order);
> > +               xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
> > +                               1 << (ctrl->write.order - PAGE_SHIFT));
> >         if (ctrl->ring) {
> >                 if (ctrl->is_server) {
> >                         ctrl->ring->srv_live = 0;
> > --
> > 1.8.1.4
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

I can't seem to find the 1/2 patch from this series anywhere?

On Sat, 2013-05-04 at 23:10 +0100, Marek Marczykowski wrote:
> On linux it will end up in munmap anyway, but do not assume any
> particular xc_gntshr_munmap implementation details.
> 
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> ---
>  tools/libvchan/io.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> index 5ec5fb9..3040099 100644
> --- a/tools/libvchan/io.c
> +++ b/tools/libvchan/io.c
> @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
>  	if (!ctrl)
>  		return;
>  	if (ctrl->read.order >= PAGE_SHIFT)
> -		munmap(ctrl->read.buffer, 1 << ctrl->read.order);
> +		xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
> +				1 << (ctrl->read.order - PAGE_SHIFT));
>  	if (ctrl->write.order >= PAGE_SHIFT)
> -		munmap(ctrl->write.buffer, 1 << ctrl->write.order);
> +		xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
> +				1 << (ctrl->write.order - PAGE_SHIFT));
>  	if (ctrl->ring) {
>  		if (ctrl->is_server) {
>  			ctrl->ring->srv_live = 0;

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Wed, 2013-05-08 at 11:57 +0100, Ian Campbell wrote:
> I can't seem to find the 1/2 patch from this series anywhere?

Looks like it might be <20130508040327.91296310@duch.mimuw.edu.pl>
"[PATCH v2 1/2] libxc: fix xc_gntshr_munmap semantic" but that was sent
nearly a week prior to this 2/2 one so I'm not at all sure.

Ian.

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Daniel De Graaf]

On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
> On linux it will end up in munmap anyway, but do not assume any
> particular xc_gntshr_munmap implementation details.
>
> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>

On a client, this ends up using xc_gntshr_munmap to unmap pages that
were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam

George: unless there is another OS besides Linux that implements the
xc_gntshr_* interfaces (I found none from a grep of the source), this
is just code clean-up and so could be postponed to 4.4.

> ---
>   tools/libvchan/io.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> index 5ec5fb9..3040099 100644
> --- a/tools/libvchan/io.c
> +++ b/tools/libvchan/io.c
> @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
>   	if (!ctrl)
>   		return;
>   	if (ctrl->read.order >= PAGE_SHIFT)
> -		munmap(ctrl->read.buffer, 1 << ctrl->read.order);
> +		xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
> +				1 << (ctrl->read.order - PAGE_SHIFT));
>   	if (ctrl->write.order >= PAGE_SHIFT)
> -		munmap(ctrl->write.buffer, 1 << ctrl->write.order);
> +		xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
> +				1 << (ctrl->write.order - PAGE_SHIFT));
>   	if (ctrl->ring) {
>   		if (ctrl->is_server) {
>   			ctrl->ring->srv_live = 0;
>
-- 
Daniel De Graaf
National Security Agency

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 551 bytes --]

On 08.05.2013 13:42, Ian Campbell wrote:
> On Wed, 2013-05-08 at 11:57 +0100, Ian Campbell wrote:
>> I can't seem to find the 1/2 patch from this series anywhere?
> 
> Looks like it might be <20130508040327.91296310@duch.mimuw.edu.pl>
> "[PATCH v2 1/2] libxc: fix xc_gntshr_munmap semantic" but that was sent
> nearly a week prior to this 2/2 one so I'm not at all sure.

Yes, this one. I've failed with git format-patch, and the mail date was from
my working tree. Sorry...

-- 
Best Regards,
Marek Marczykowski
Invisible Things Lab


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 1850 bytes --]

On 08.05.2013 15:49, Daniel De Graaf wrote:
> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
>> On linux it will end up in munmap anyway, but do not assume any
>> particular xc_gntshr_munmap implementation details.
>>
>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> 
> On a client, this ends up using xc_gntshr_munmap to unmap pages that
> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
> 
> George: unless there is another OS besides Linux that implements the
> xc_gntshr_* interfaces (I found none from a grep of the source), this
> is just code clean-up and so could be postponed to 4.4.

This is actually prerequirement for libvchan for mini-os (already posted v1,
working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
can wait.

> 
>> ---
>>   tools/libvchan/io.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
>> index 5ec5fb9..3040099 100644
>> --- a/tools/libvchan/io.c
>> +++ b/tools/libvchan/io.c
>> @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
>>       if (!ctrl)
>>           return;
>>       if (ctrl->read.order >= PAGE_SHIFT)
>> -        munmap(ctrl->read.buffer, 1 << ctrl->read.order);
>> +        xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
>> +                1 << (ctrl->read.order - PAGE_SHIFT));
>>       if (ctrl->write.order >= PAGE_SHIFT)
>> -        munmap(ctrl->write.buffer, 1 << ctrl->write.order);
>> +        xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
>> +                1 << (ctrl->write.order - PAGE_SHIFT));
>>       if (ctrl->ring) {
>>           if (ctrl->is_server) {
>>               ctrl->ring->srv_live = 0;
>>


-- 
Best Regards,
Marek Marczykowski
Invisible Things Lab


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
> On 08.05.2013 15:49, Daniel De Graaf wrote:
> > On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
> >> On linux it will end up in munmap anyway, but do not assume any
> >> particular xc_gntshr_munmap implementation details.
> >>
> >> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> > 
> > On a client, this ends up using xc_gntshr_munmap to unmap pages that
> > were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
> > 
> > George: unless there is another OS besides Linux that implements the
> > xc_gntshr_* interfaces (I found none from a grep of the source), this
> > is just code clean-up and so could be postponed to 4.4.
> 
> This is actually prerequirement for libvchan for mini-os (already posted v1,
> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
> can wait.

Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
in this assessment?

Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 1458 bytes --]

On 10.05.2013 15:46, Ian Campbell wrote:
> On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
>> On 08.05.2013 15:49, Daniel De Graaf wrote:
>>> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
>>>> On linux it will end up in munmap anyway, but do not assume any
>>>> particular xc_gntshr_munmap implementation details.
>>>>
>>>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
>>>
>>> On a client, this ends up using xc_gntshr_munmap to unmap pages that
>>> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
>>>
>>> George: unless there is another OS besides Linux that implements the
>>> xc_gntshr_* interfaces (I found none from a grep of the source), this
>>> is just code clean-up and so could be postponed to 4.4.
>>
>> This is actually prerequirement for libvchan for mini-os (already posted v1,
>> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
>> can wait.
> 
> Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
> in this assessment?
> 
> Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>

As long as libxenvchan is the only (at least I'm aware of) user of
xc_gntshr_munmap - probably yes. But IMO it's better to fix xc_gntshr_munmap
sooner than later, to minimize changes in code that uses it (if someone will
start to use it before 4.4 release).

-- 
Best Regards,
Marek Marczykowski
Invisible Things Lab


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[George Dunlap]

On 10/05/13 14:55, Marek Marczykowski wrote:
> On 10.05.2013 15:46, Ian Campbell wrote:
>> On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
>>> On 08.05.2013 15:49, Daniel De Graaf wrote:
>>>> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
>>>>> On linux it will end up in munmap anyway, but do not assume any
>>>>> particular xc_gntshr_munmap implementation details.
>>>>>
>>>>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
>>>> On a client, this ends up using xc_gntshr_munmap to unmap pages that
>>>> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
>>>>
>>>> George: unless there is another OS besides Linux that implements the
>>>> xc_gntshr_* interfaces (I found none from a grep of the source), this
>>>> is just code clean-up and so could be postponed to 4.4.
>>> This is actually prerequirement for libvchan for mini-os (already posted v1,
>>> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
>>> can wait.
>> Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
>> in this assessment?
>>
>> Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>
> As long as libxenvchan is the only (at least I'm aware of) user of
> xc_gntshr_munmap - probably yes. But IMO it's better to fix xc_gntshr_munmap
> sooner than later, to minimize changes in code that uses it (if someone will
> start to use it before 4.4 release).

I take it from the name that xc_grntshr_munmap() is in libxc?  No one 
externally should be linking against libxc; it's an internal library 
without a stable interface.

  -George

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Fri, 2013-05-10 at 14:55 +0100, Marek Marczykowski wrote:
> On 10.05.2013 15:46, Ian Campbell wrote:
> > On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
> >> On 08.05.2013 15:49, Daniel De Graaf wrote:
> >>> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
> >>>> On linux it will end up in munmap anyway, but do not assume any
> >>>> particular xc_gntshr_munmap implementation details.
> >>>>
> >>>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> >>>
> >>> On a client, this ends up using xc_gntshr_munmap to unmap pages that
> >>> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
> >>>
> >>> George: unless there is another OS besides Linux that implements the
> >>> xc_gntshr_* interfaces (I found none from a grep of the source), this
> >>> is just code clean-up and so could be postponed to 4.4.
> >>
> >> This is actually prerequirement for libvchan for mini-os (already posted v1,
> >> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
> >> can wait.
> > 
> > Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
> > in this assessment?
> > 
> > Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>
> 
> As long as libxenvchan is the only (at least I'm aware of) user of
> xc_gntshr_munmap - probably yes. But IMO it's better to fix xc_gntshr_munmap
> sooner than later, to minimize changes in code that uses it (if someone will
> start to use it before 4.4 release).

I'm not sure if you are arguing for or against putting that fix into
4.3?

Ian.

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[George Dunlap]

On 10/05/13 15:08, Ian Campbell wrote:
> On Fri, 2013-05-10 at 14:55 +0100, Marek Marczykowski wrote:
>> On 10.05.2013 15:46, Ian Campbell wrote:
>>> On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
>>>> On 08.05.2013 15:49, Daniel De Graaf wrote:
>>>>> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
>>>>>> On linux it will end up in munmap anyway, but do not assume any
>>>>>> particular xc_gntshr_munmap implementation details.
>>>>>>
>>>>>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
>>>>> On a client, this ends up using xc_gntshr_munmap to unmap pages that
>>>>> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
>>>>>
>>>>> George: unless there is another OS besides Linux that implements the
>>>>> xc_gntshr_* interfaces (I found none from a grep of the source), this
>>>>> is just code clean-up and so could be postponed to 4.4.
>>>> This is actually prerequirement for libvchan for mini-os (already posted v1,
>>>> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
>>>> can wait.
>>> Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
>>> in this assessment?
>>>
>>> Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>
>> As long as libxenvchan is the only (at least I'm aware of) user of
>> xc_gntshr_munmap - probably yes. But IMO it's better to fix xc_gntshr_munmap
>> sooner than later, to minimize changes in code that uses it (if someone will
>> start to use it before 4.4 release).
> I'm not sure if you are arguing for or against putting that fix into
> 4.3?

I read this as, "We should make the interface more useful in case 
someone decides to use it after 4.3 comes out but before 4.4 comes 
out."  To which my response was, if this is in libxc, people shouldn't 
be linking against it anyway.

  -George

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Fri, 2013-05-10 at 15:15 +0100, George Dunlap wrote:
> On 10/05/13 15:08, Ian Campbell wrote:
> > On Fri, 2013-05-10 at 14:55 +0100, Marek Marczykowski wrote:
> >> On 10.05.2013 15:46, Ian Campbell wrote:
> >>> On Wed, 2013-05-08 at 15:08 +0100, Marek Marczykowski wrote:
> >>>> On 08.05.2013 15:49, Daniel De Graaf wrote:
> >>>>> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
> >>>>>> On linux it will end up in munmap anyway, but do not assume any
> >>>>>> particular xc_gntshr_munmap implementation details.
> >>>>>>
> >>>>>> Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> >>>>> On a client, this ends up using xc_gntshr_munmap to unmap pages that
> >>>>> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam
> >>>>>
> >>>>> George: unless there is another OS besides Linux that implements the
> >>>>> xc_gntshr_* interfaces (I found none from a grep of the source), this
> >>>>> is just code clean-up and so could be postponed to 4.4.
> >>>> This is actually prerequirement for libvchan for mini-os (already posted v1,
> >>>> working on v2). But as mini-os libvchan isn't targeted for 4.3, this one also
> >>>> can wait.
> >>> Is the first patch "libxc: fix xc_gntshr_munmap semantic" also included
> >>> in this assessment?
> >>>
> >>> Message-Id: <20130508040327.91296310@duch.mimuw.edu.pl>
> >> As long as libxenvchan is the only (at least I'm aware of) user of
> >> xc_gntshr_munmap - probably yes. But IMO it's better to fix xc_gntshr_munmap
> >> sooner than later, to minimize changes in code that uses it (if someone will
> >> start to use it before 4.4 release).
> > I'm not sure if you are arguing for or against putting that fix into
> > 4.3?
> 
> I read this as, "We should make the interface more useful in case 
> someone decides to use it after 4.3 comes out but before 4.4 comes 
> out."  To which my response was, if this is in libxc, people shouldn't 
> be linking against it anyway.

OK. Strictly speaking you mean "shouldn't rely on the libxc API not
changing", we don't forbid people to link against libxc.

Ian.

Re: [PATCH v2 2/2] libvchan: replace munmap with correct xc_gntshr_munmap

[Ian Campbell]

On Wed, 2013-05-08 at 14:49 +0100, Daniel De Graaf wrote:
> On 05/04/2013 06:10 PM, Marek Marczykowski wrote:
> > On linux it will end up in munmap anyway, but do not assume any
> > particular xc_gntshr_munmap implementation details.
> >
> > Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> 
> On a client, this ends up using xc_gntshr_munmap to unmap pages that
> were mapped with xc_gnttab_map_* instead of using xc_gnttab_unmam

Did this aspect get resolved?

I suppose I shouldn't apply the 1/2 patch either until this is sorted?

> 
> George: unless there is another OS besides Linux that implements the
> xc_gntshr_* interfaces (I found none from a grep of the source), this
> is just code clean-up and so could be postponed to 4.4.
> 
> > ---
> >   tools/libvchan/io.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libvchan/io.c b/tools/libvchan/io.c
> > index 5ec5fb9..3040099 100644
> > --- a/tools/libvchan/io.c
> > +++ b/tools/libvchan/io.c
> > @@ -324,9 +324,11 @@ void libxenvchan_close(struct libxenvchan *ctrl)
> >   	if (!ctrl)
> >   		return;
> >   	if (ctrl->read.order >= PAGE_SHIFT)
> > -		munmap(ctrl->read.buffer, 1 << ctrl->read.order);
> > +		xc_gntshr_munmap(ctrl->gntshr, ctrl->read.buffer,
> > +				1 << (ctrl->read.order - PAGE_SHIFT));
> >   	if (ctrl->write.order >= PAGE_SHIFT)
> > -		munmap(ctrl->write.buffer, 1 << ctrl->write.order);
> > +		xc_gntshr_munmap(ctrl->gntshr, ctrl->write.buffer,
> > +				1 << (ctrl->write.order - PAGE_SHIFT));
> >   	if (ctrl->ring) {
> >   		if (ctrl->is_server) {
> >   			ctrl->ring->srv_live = 0;
> >