Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1548534 > unrolled thread
| Started by | Kenneth Lee <liguozhu@hisilicon.com> |
|---|---|
| First post | 2016-12-30 12:00 +0100 |
| Last post | 2017-01-04 08:10 +0100 |
| Articles | 7 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v3] IB/umem: Release pid in error and ODP flow Kenneth Lee <liguozhu@hisilicon.com> - 2016-12-30 12:00 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Leon Romanovsky <leon@kernel.org> - 2017-01-01 07:50 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Kenneth Lee <liguozhu@hisilicon.com> - 2017-01-03 02:50 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Kenneth Lee <liguozhu@hisilicon.com> - 2017-01-03 03:10 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Leon Romanovsky <leon@kernel.org> - 2017-01-03 11:20 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Kenneth Lee <liguozhu@hisilicon.com> - 2017-01-04 07:40 +0100
Re: [PATCH v3] IB/umem: Release pid in error and ODP flow Leon Romanovsky <leon@kernel.org> - 2017-01-04 08:10 +0100
| From | Kenneth Lee <liguozhu@hisilicon.com> |
|---|---|
| Date | 2016-12-30 12:00 +0100 |
| Subject | [PATCH v3] IB/umem: Release pid in error and ODP flow |
| Message-ID | <sU34K-3gb-19@gated-at.bofh.it> |
There are two bugfixes in this patch:
Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
This patch introduce the get_task_pid but not put it back on all error
path
Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
This patch introduce a ODP flow without release pid before enter it
Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
Reviewed-by: Haggai Eran <haggaie@mellanox.com>
---
Change from v1 to v2:
Correcting the patch title and description
Change from v2 to v3:
Update the title and add "Fixes" fields in the description
drivers/infiniband/core/umem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index 1e62a5f..4609b92 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
if (access & IB_ACCESS_ON_DEMAND) {
+ put_pid(umem->pid);
ret = ib_umem_odp_get(context, umem);
if (ret) {
kfree(umem);
@@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
page_list = (struct page **) __get_free_page(GFP_KERNEL);
if (!page_list) {
+ put_pid(umem->pid);
kfree(umem);
return ERR_PTR(-ENOMEM);
}
--
1.9.1
[toc] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2017-01-01 07:50 +0100 |
| Message-ID | <sUI7T-5lK-1@gated-at.bofh.it> |
| In reply to | #1548534 |
[Multipart message — attachments visible in raw view] — view raw
On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> There are two bugfixes in this patch:
>
> Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> This patch introduce the get_task_pid but not put it back on all error
> path
>
> Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> This patch introduce a ODP flow without release pid before enter it
>
>
> Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> ---
> Change from v1 to v2:
> Correcting the patch title and description
> Change from v2 to v3:
> Update the title and add "Fixes" fields in the description
OK,
I see that you still didn't read Documentation/SubmittingPatches. You
must read that document before you are sending patches.
But I'll stop here, the code is correct (it fixes bugs) and commit message
more usefull than before.
>
> drivers/infiniband/core/umem.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 1e62a5f..4609b92 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
>
> if (access & IB_ACCESS_ON_DEMAND) {
> + put_pid(umem->pid);
> ret = ib_umem_odp_get(context, umem);
> if (ret) {
> kfree(umem);
> @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
>
> page_list = (struct page **) __get_free_page(GFP_KERNEL);
> if (!page_list) {
> + put_pid(umem->pid);
> kfree(umem);
> return ERR_PTR(-ENOMEM);
> }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[toc] | [prev] | [next] | [standalone]
| From | Kenneth Lee <liguozhu@hisilicon.com> |
|---|---|
| Date | 2017-01-03 02:50 +0100 |
| Message-ID | <sVmoF-7jY-3@gated-at.bofh.it> |
| In reply to | #1548864 |
Thanks,
I did read the doc, but maybe I mis-understant some points. Could you please
point it out?
On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> Date: Sun, 1 Jan 2017 08:47:12 +0200
> From: Leon Romanovsky <leon@kernel.org>
> To: Kenneth Lee <liguozhu@hisilicon.com>
> CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
>
> On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > There are two bugfixes in this patch:
> >
> > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> > This patch introduce the get_task_pid but not put it back on all error
> > path
> >
> > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> > This patch introduce a ODP flow without release pid before enter it
> >
> >
> > Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> > Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> > ---
> > Change from v1 to v2:
> > Correcting the patch title and description
> > Change from v2 to v3:
> > Update the title and add "Fixes" fields in the description
>
> OK,
>
> I see that you still didn't read Documentation/SubmittingPatches. You
> must read that document before you are sending patches.
>
> But I'll stop here, the code is correct (it fixes bugs) and commit message
> more usefull than before.
>
>
> >
> > drivers/infiniband/core/umem.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > index 1e62a5f..4609b92 100644
> > --- a/drivers/infiniband/core/umem.c
> > +++ b/drivers/infiniband/core/umem.c
> > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> >
> > if (access & IB_ACCESS_ON_DEMAND) {
> > + put_pid(umem->pid);
> > ret = ib_umem_odp_get(context, umem);
> > if (ret) {
> > kfree(umem);
> > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> >
> > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > if (!page_list) {
> > + put_pid(umem->pid);
> > kfree(umem);
> > return ERR_PTR(-ENOMEM);
> > }
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
-Kenneth(Hisilicon)
[toc] | [prev] | [next] | [standalone]
| From | Kenneth Lee <liguozhu@hisilicon.com> |
|---|---|
| Date | 2017-01-03 03:10 +0100 |
| Message-ID | <sVmI2-7Fn-5@gated-at.bofh.it> |
| In reply to | #1548864 |
On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> Date: Sun, 1 Jan 2017 08:47:12 +0200
> From: Leon Romanovsky <leon@kernel.org>
> To: Kenneth Lee <liguozhu@hisilicon.com>
> CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
>
> On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > There are two bugfixes in this patch:
> >
> > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> > This patch introduce the get_task_pid but not put it back on all error
> > path
> >
> > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> > This patch introduce a ODP flow without release pid before enter it
> >
> >
> > Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> > Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> > ---
> > Change from v1 to v2:
> > Correcting the patch title and description
> > Change from v2 to v3:
> > Update the title and add "Fixes" fields in the description
>
> OK,
>
> I see that you still didn't read Documentation/SubmittingPatches. You
> must read that document before you are sending patches.
>
> But I'll stop here, the code is correct (it fixes bugs) and commit message
> more usefull than before.
>
>
> >
> > drivers/infiniband/core/umem.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > index 1e62a5f..4609b92 100644
> > --- a/drivers/infiniband/core/umem.c
> > +++ b/drivers/infiniband/core/umem.c
> > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> >
> > if (access & IB_ACCESS_ON_DEMAND) {
> > + put_pid(umem->pid);
> > ret = ib_umem_odp_get(context, umem);
> > if (ret) {
> > kfree(umem);
> > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> >
> > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > if (!page_list) {
> > + put_pid(umem->pid);
> > kfree(umem);
> > return ERR_PTR(-ENOMEM);
> > }
> > --
> > 1.9.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks,
I did read the doc, but maybe I mis-understant some points. Could you please
point it out?
And sorry. please ignore the last message. I forget to use a bottom-post style.
--
-Kenneth(Hisilicon)
[toc] | [prev] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2017-01-03 11:20 +0100 |
| Message-ID | <sVume-4Og-9@gated-at.bofh.it> |
| In reply to | #1549498 |
[Multipart message — attachments visible in raw view] — view raw
On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > From: Leon Romanovsky <leon@kernel.org>
> > To: Kenneth Lee <liguozhu@hisilicon.com>
> > CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> > robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> > vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> > krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> > linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> >
> > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > There are two bugfixes in this patch:
> > >
> > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> > > This patch introduce the get_task_pid but not put it back on all error
> > > path
> > >
> > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> > > This patch introduce a ODP flow without release pid before enter it
> > >
> > >
> > > Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> > > Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> > > ---
> > > Change from v1 to v2:
> > > Correcting the patch title and description
> > > Change from v2 to v3:
> > > Update the title and add "Fixes" fields in the description
> >
> > OK,
> >
> > I see that you still didn't read Documentation/SubmittingPatches. You
> > must read that document before you are sending patches.
> >
> > But I'll stop here, the code is correct (it fixes bugs) and commit message
> > more usefull than before.
> >
> >
> > >
> > > drivers/infiniband/core/umem.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > > index 1e62a5f..4609b92 100644
> > > --- a/drivers/infiniband/core/umem.c
> > > +++ b/drivers/infiniband/core/umem.c
> > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > > IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > >
> > > if (access & IB_ACCESS_ON_DEMAND) {
> > > + put_pid(umem->pid);
> > > ret = ib_umem_odp_get(context, umem);
> > > if (ret) {
> > > kfree(umem);
> > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > >
> > > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > if (!page_list) {
> > > + put_pid(umem->pid);
> > > kfree(umem);
> > > return ERR_PTR(-ENOMEM);
> > > }
> > > --
> > > 1.9.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Thanks,
>
> I did read the doc, but maybe I mis-understant some points. Could you please
> point it out?
Fixes line should be placed above bottom signatures.
As an example of properly written patch, you can take a look on the
following patch [1] from Steve.
[1] http://marc.info/?l=linux-rdma&m=148244272205411&w=2
>
> And sorry. please ignore the last message. I forget to use a bottom-post style.
>
>
>
> --
> -Kenneth(Hisilicon)
[toc] | [prev] | [next] | [standalone]
| From | Kenneth Lee <liguozhu@hisilicon.com> |
|---|---|
| Date | 2017-01-04 07:40 +0100 |
| Message-ID | <sVNoR-RY-15@gated-at.bofh.it> |
| In reply to | #1549728 |
On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> Date: Tue, 3 Jan 2017 12:12:24 +0200
> From: Leon Romanovsky <leon@kernel.org>
> To: Kenneth Lee <liguozhu@hisilicon.com>
> CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20170103101224.GH12077@mtr-leonro.local>
>
> On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > From: Leon Romanovsky <leon@kernel.org>
> > > To: Kenneth Lee <liguozhu@hisilicon.com>
> > > CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> > > robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> > > vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> > > krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> > > linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > >
> > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > There are two bugfixes in this patch:
> > > >
> > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> > > > This patch introduce the get_task_pid but not put it back on all error
> > > > path
> > > >
> > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> > > > This patch introduce a ODP flow without release pid before enter it
> > > >
> > > >
> > > > Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> > > > Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> > > > ---
> > > > Change from v1 to v2:
> > > > Correcting the patch title and description
> > > > Change from v2 to v3:
> > > > Update the title and add "Fixes" fields in the description
> > >
> > > OK,
> > >
> > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > must read that document before you are sending patches.
> > >
> > > But I'll stop here, the code is correct (it fixes bugs) and commit message
> > > more usefull than before.
> > >
> > >
> > > >
> > > > drivers/infiniband/core/umem.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > > > index 1e62a5f..4609b92 100644
> > > > --- a/drivers/infiniband/core/umem.c
> > > > +++ b/drivers/infiniband/core/umem.c
> > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > > > IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > >
> > > > if (access & IB_ACCESS_ON_DEMAND) {
> > > > + put_pid(umem->pid);
> > > > ret = ib_umem_odp_get(context, umem);
> > > > if (ret) {
> > > > kfree(umem);
> > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > > >
> > > > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > if (!page_list) {
> > > > + put_pid(umem->pid);
> > > > kfree(umem);
> > > > return ERR_PTR(-ENOMEM);
> > > > }
> > > > --
> > > > 1.9.1
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Thanks,
> >
> > I did read the doc, but maybe I mis-understant some points. Could you please
> > point it out?
>
> Fixes line should be placed above bottom signatures.
>
> As an example of properly written patch, you can take a look on the
> following patch [1] from Steve.
>
> [1] http://marc.info/?l=linux-rdma&m=148244272205411&w=2
Thank you. A sample help a lot.
But please allow me to argue a little:
Documentation/process/submitting-patches.rst does really not mention where Fixes
tags should be put:)
>
> >
> > And sorry. please ignore the last message. I forget to use a bottom-post style.
> >
> >
> >
> > --
> > -Kenneth(Hisilicon)
--
-Kenneth(Hisilicon)
[toc] | [prev] | [next] | [standalone]
| From | Leon Romanovsky <leon@kernel.org> |
|---|---|
| Date | 2017-01-04 08:10 +0100 |
| Message-ID | <sVNRT-1io-23@gated-at.bofh.it> |
| In reply to | #1550464 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, Jan 04, 2017 at 03:07:04PM +0800, Kenneth Lee wrote:
> On Tue, Jan 03, 2017 at 12:12:24PM +0200, Leon Romanovsky wrote:
> > Date: Tue, 3 Jan 2017 12:12:24 +0200
> > From: Leon Romanovsky <leon@kernel.org>
> > To: Kenneth Lee <liguozhu@hisilicon.com>
> > CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> > robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> > vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> > krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> > linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20170103101224.GH12077@mtr-leonro.local>
> >
> > On Tue, Jan 03, 2017 at 10:32:50AM +0800, Kenneth Lee wrote:
> > > On Sun, Jan 01, 2017 at 08:47:12AM +0200, Leon Romanovsky wrote:
> > > > Date: Sun, 1 Jan 2017 08:47:12 +0200
> > > > From: Leon Romanovsky <leon@kernel.org>
> > > > To: Kenneth Lee <liguozhu@hisilicon.com>
> > > > CC: dledford@redhat.com, sean.hefty@intel.com, hal.rosenstock@gmail.com,
> > > > robin.murphy@arm.com, jroedel@suse.de, egtvedt@samfundet.no,
> > > > vgupta@synopsys.com, dave.hansen@linux.intel.com, lstoakes@gmail.com,
> > > > krzk@kernel.org, sebott@linux.vnet.ibm.com, markb@mellanox.com,
> > > > linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
> > > > Subject: Re: [PATCH v3] IB/umem: Release pid in error and ODP flow
> > > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > > Message-ID: <20170101064712.GQ26885@mtr-leonro.local>
> > > >
> > > > On Fri, Dec 30, 2016 at 06:18:29PM +0800, Kenneth Lee wrote:
> > > > > There are two bugfixes in this patch:
> > > > >
> > > > > Fixes: 87773dd56d54 ("IB: ib_umem_release() should decrement mm->pinned_vm from ib_umem_get")
> > > > > This patch introduce the get_task_pid but not put it back on all error
> > > > > path
> > > > >
> > > > > Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> > > > > This patch introduce a ODP flow without release pid before enter it
> > > > >
> > > > >
> > > > > Signed-off-by: Kenneth Lee <liguozhu@hisilicon.com>
> > > > > Reviewed-by: Haggai Eran <haggaie@mellanox.com>
> > > > > ---
> > > > > Change from v1 to v2:
> > > > > Correcting the patch title and description
> > > > > Change from v2 to v3:
> > > > > Update the title and add "Fixes" fields in the description
> > > >
> > > > OK,
> > > >
> > > > I see that you still didn't read Documentation/SubmittingPatches. You
> > > > must read that document before you are sending patches.
> > > >
> > > > But I'll stop here, the code is correct (it fixes bugs) and commit message
> > > > more usefull than before.
> > > >
> > > >
> > > > >
> > > > > drivers/infiniband/core/umem.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > > > > index 1e62a5f..4609b92 100644
> > > > > --- a/drivers/infiniband/core/umem.c
> > > > > +++ b/drivers/infiniband/core/umem.c
> > > > > @@ -134,6 +134,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > > > > IB_ACCESS_REMOTE_ATOMIC | IB_ACCESS_MW_BIND));
> > > > >
> > > > > if (access & IB_ACCESS_ON_DEMAND) {
> > > > > + put_pid(umem->pid);
> > > > > ret = ib_umem_odp_get(context, umem);
> > > > > if (ret) {
> > > > > kfree(umem);
> > > > > @@ -149,6 +150,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr,
> > > > >
> > > > > page_list = (struct page **) __get_free_page(GFP_KERNEL);
> > > > > if (!page_list) {
> > > > > + put_pid(umem->pid);
> > > > > kfree(umem);
> > > > > return ERR_PTR(-ENOMEM);
> > > > > }
> > > > > --
> > > > > 1.9.1
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > > Thanks,
> > >
> > > I did read the doc, but maybe I mis-understant some points. Could you please
> > > point it out?
> >
> > Fixes line should be placed above bottom signatures.
> >
> > As an example of properly written patch, you can take a look on the
> > following patch [1] from Steve.
> >
> > [1] http://marc.info/?l=linux-rdma&m=148244272205411&w=2
>
> Thank you. A sample help a lot.
>
> But please allow me to argue a little:
> Documentation/process/submitting-patches.rst does really not mention where Fixes
> tags should be put:)
Sure, you can't and don't want to document everything. There is section named
"2) Describe your changes" which has very extensive description about
commit message. If you had followed it, you would find that it is very
natural to place Fixes at the bottom.
Thanks
>
> >
> > >
> > > And sorry. please ignore the last message. I forget to use a bottom-post style.
> > >
> > >
> > >
> > > --
> > > -Kenneth(Hisilicon)
>
>
>
> --
> -Kenneth(Hisilicon)
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web