[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[Mauricio Vásquez]

Dear All,

I was looking at that patch, I can understand its functionality but not its
implementation..

Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would
not be sufficient? After all, the goal is to mark the memzone as used by
ivshmem to forbid  freeing it.

Please corrected if I am missing something.

Thank you,

Mauricio Vasquez,

Re: [dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[Burakov, Anatoly]

Hi Mauricio,

You’re not missing anything. It would be done this way, if the memzone parameter wasn’t const. But it is const, so we have to find the memzone in config to edit it.

Thanks,
Anatoly

From: Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it]
Sent: Wednesday, March 30, 2016 9:22 AM
To: dev@dpdk.org
Cc: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>
Subject: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Dear All,
I was looking at that patch, I can understand its functionality but not its implementation..
Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would not be sufficient? After all, the goal is to mark the memzone as used by ivshmem to forbid  freeing it.
Please corrected if I am missing something.
Thank you,
Mauricio Vasquez,

Re: [dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[Mauricio Vásquez]

Hi Anatoly,

Thank you very much, I did not take into account that detail.

I have two additional concerns about it:

1. I think it is possible to have a race condition. The memzone is marked
as not freeable after it has been added to the ivshmem device, then it is
possible to free the memzone just after it has been  added to the metadata
but before it is marked as not freeable.
Shouldn't the memzone be marked before adding the memzone to the ivshmem
device?

2. Are the #ifdefs necessary?, we already are in  a file that will only
compiled when ivshmem is enabled.

Thanks,

Mauricio Vasquez,


On Wed, Mar 30, 2016 at 10:29 AM, Burakov, Anatoly <
anatoly.burakov@intel.com> wrote:

> Hi Mauricio,
>
>
>
> You’re not missing anything. It would be done this way, if the memzone
> parameter wasn’t const. But it is const, so we have to find the memzone in
> config to edit it.
>
>
>
> Thanks,
>
> Anatoly
>
>
>
> *From:* Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it]
>
> *Sent:* Wednesday, March 30, 2016 9:22 AM
> *To:* dev@dpdk.org
> *Cc:* Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Burakov, Anatoly <
> anatoly.burakov@intel.com>
> *Subject:* Question about cd10c42eb5bc ("mem: fix ivshmem freeing")
>
>
>
> Dear All,
>
> I was looking at that patch, I can understand its functionality but not
> its implementation..
>
> Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would
> not be sufficient? After all, the goal is to mark the memzone as used by
> ivshmem to forbid  freeing it.
>
> Please corrected if I am missing something.
>
> Thank you,
>
> Mauricio Vasquez,
>

Re: [dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[Burakov, Anatoly]

Hi Mauricio,

Good points. Would you be willing to prepare a patch to fix these issues?

Thanks,
Anatoly

From: Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it]
Sent: Wednesday, March 30, 2016 10:13 AM
To: Burakov, Anatoly <anatoly.burakov@intel.com>
Cc: dev@dpdk.org; Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>
Subject: Re: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Hi Anatoly,
Thank you very much, I did not take into account that detail.
I have two additional concerns about it:
1. I think it is possible to have a race condition. The memzone is marked as not freeable after it has been added to the ivshmem device, then it is possible to free the memzone just after it has been  added to the metadata but before it is marked as not freeable.
Shouldn't the memzone be marked before adding the memzone to the ivshmem device?

2. Are the #ifdefs necessary?, we already are in  a file that will only compiled when ivshmem is enabled.
Thanks,
Mauricio Vasquez,


On Wed, Mar 30, 2016 at 10:29 AM, Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>> wrote:
Hi Mauricio,

You’re not missing anything. It would be done this way, if the memzone parameter wasn’t const. But it is const, so we have to find the memzone in config to edit it.

Thanks,
Anatoly

From: Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it<mailto:mauricio.vasquezbernal@studenti.polito.it>]
Sent: Wednesday, March 30, 2016 9:22 AM
To: dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com<mailto:sergio.gonzalez.monroy@intel.com>>; Richardson, Bruce <bruce.richardson@intel.com<mailto:bruce.richardson@intel.com>>; Burakov, Anatoly <anatoly.burakov@intel.com<mailto:anatoly.burakov@intel.com>>
Subject: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

Dear All,
I was looking at that patch, I can understand its functionality but not its implementation..
Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would not be sufficient? After all, the goal is to mark the memzone as used by ivshmem to forbid  freeing it.
Please corrected if I am missing something.
Thank you,
Mauricio Vasquez,

Re: [dpdk-dev] Question about cd10c42eb5bc ("mem: fix ivshmem freeing")

[Mauricio Vásquez]

Sure: http://dpdk.org/ml/archives/dev/2016-April/036884.html,

On Wed, Mar 30, 2016 at 2:10 PM, Burakov, Anatoly <anatoly.burakov@intel.com
> wrote:

> Hi Mauricio,
>
>
>
> Good points. Would you be willing to prepare a patch to fix these issues?
>
>
>
> Thanks,
>
> Anatoly
>
>
>
> *From:* Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it]
>
> *Sent:* Wednesday, March 30, 2016 10:13 AM
> *To:* Burakov, Anatoly <anatoly.burakov@intel.com>
> *Cc:* dev@dpdk.org; Gonzalez Monroy, Sergio <
> sergio.gonzalez.monroy@intel.com>; Richardson, Bruce <
> bruce.richardson@intel.com>
> *Subject:* Re: Question about cd10c42eb5bc ("mem: fix ivshmem freeing")
>
>
>
> Hi Anatoly,
>
> Thank you very much, I did not take into account that detail.
>
> I have two additional concerns about it:
>
> 1. I think it is possible to have a race condition. The memzone is marked
> as not freeable after it has been added to the ivshmem device, then it is
> possible to free the memzone just after it has been  added to the metadata
> but before it is marked as not freeable.
>
> Shouldn't the memzone be marked before adding the memzone to the ivshmem
> device?
>
>
>
> 2. Are the #ifdefs necessary?, we already are in  a file that will only
> compiled when ivshmem is enabled.
>
> Thanks,
>
> Mauricio Vasquez,
>
>
>
>
>
> On Wed, Mar 30, 2016 at 10:29 AM, Burakov, Anatoly <
> anatoly.burakov@intel.com> wrote:
>
> Hi Mauricio,
>
>
>
> You’re not missing anything. It would be done this way, if the memzone
> parameter wasn’t const. But it is const, so we have to find the memzone in
> config to edit it.
>
>
>
> Thanks,
>
> Anatoly
>
>
>
> *From:* Mauricio Vásquez [mailto:mauricio.vasquezbernal@studenti.polito.it]
>
> *Sent:* Wednesday, March 30, 2016 9:22 AM
> *To:* dev@dpdk.org
> *Cc:* Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Burakov, Anatoly <
> anatoly.burakov@intel.com>
> *Subject:* Question about cd10c42eb5bc ("mem: fix ivshmem freeing")
>
>
>
> Dear All,
>
> I was looking at that patch, I can understand its functionality but not
> its implementation..
>
> Why to calculate idx?, Just doing "mz->ioremap_addr = mz->phys_addr" would
> not be sufficient? After all, the goal is to mark the memzone as used by
> ivshmem to forbid  freeing it.
>
> Please corrected if I am missing something.
>
> Thank you,
>
> Mauricio Vasquez,
>
>
>