Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1548228 > unrolled thread

[PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get() Kenneth Lee <liguozhu@hisilicon.com> - 2016-12-29 09:10 +0100
    Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get() Leon Romanovsky <leon@kernel.org> - 2016-12-29 09:20 +0100
      Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get() Kenneth Lee <liguozhu@hisilicon.com> - 2016-12-30 05:30 +0100
        Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get() Leon Romanovsky <leon@kernel.org> - 2016-12-30 08:00 +0100
          Re: [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get() Kenneth Lee <liguozhu@hisilicon.com> - 2016-12-30 11:00 +0100

#1548228 — [PATCH v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()

There are two bugfixes in this patch:

1. When the execution go to the ib_umem_odp_get() path, pid should be put
   back.
2. When the memory allocation fail, the pid also should be put back before
   exit.

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

 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]

#1548233

[Multipart message — attachments visible in raw view] — view raw

On Thu, Dec 29, 2016 at 04:27:28PM +0800, Kenneth Lee wrote:
> There are two bugfixes in this patch:
>
> 1. When the execution go to the ib_umem_odp_get() path, pid should be put
>    back.
> 2. When the memory allocation fail, the pid also should be put back before
>    exit.
>
> 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

I don't see any changes except version in the title.
What about anything like this?
[PATCH v3] IB/umem: Release pid in error and ODP flows

And Fixes line please, it will help to forward it to stable trees.

Thanks

[toc] | [prev] | [next] | [standalone]

#1548459

Hi, Leon,

1. I do change the title except for the version number itself:) But my English
is quite bad, maybe the title is still quite stupid. I can update it according
to your advice.

2. I catched the bug by reading the final code, not by bisect-ing the old
commit. Do you means I should find out which commit introducing the bug? It will
not be easily to say which it is because it is a "missing bug", rather than a
"introduced bug". Indicate the commit may not help to remove a patch/commit from
the stable tree.

Could you please give more suggestion? Thanks.

On Thu, Dec 29, 2016 at 10:17:56AM +0200, Leon Romanovsky wrote:
> Date: Thu, 29 Dec 2016 10:17:56 +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 v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20161229081756.GI26885@mtr-leonro.local>
> 
> On Thu, Dec 29, 2016 at 04:27:28PM +0800, Kenneth Lee wrote:
> > There are two bugfixes in this patch:
> >
> > 1. When the execution go to the ib_umem_odp_get() path, pid should be put
> >    back.
> > 2. When the memory allocation fail, the pid also should be put back before
> >    exit.
> >
> > 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
> 
> I don't see any changes except version in the title.
> What about anything like this?
> [PATCH v3] IB/umem: Release pid in error and ODP flows
> 
> And Fixes line please, it will help to forward it to stable trees.
> 
> Thanks



-- 
			-Kenneth(Hisilicon)

[toc] | [prev] | [next] | [standalone]

#1548471

[Multipart message — attachments visible in raw view] — view raw

On Fri, Dec 30, 2016 at 12:50:11PM +0800, Kenneth Lee wrote:
> Hi, Leon,
>
> 1. I do change the title except for the version number itself:) But my English
> is quite bad, maybe the title is still quite stupid. I can update it according
> to your advice.

Yes, please
The main points are:
1. Remove "bugifix", it is not needed.
2. Use description in the title and not function names.

>
> 2. I catched the bug by reading the final code, not by bisect-ing the old
> commit. Do you means I should find out which commit introducing the bug? It will
> not be easily to say which it is because it is a "missing bug", rather than a
> "introduced bug". Indicate the commit may not help to remove a patch/commit from
> the stable tree.

The fixes line won't cause for removal of commit, but to addition of
yours on top of their code base.

git blame is your friend.

one fixes line is:
Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")

and the second line is !!!!! NOT !!!!!, you need to go deeper in the logs !!!!!!
Fixes: f7c6a7b5d599 ("IB/uverbs: Export ib_umem_get()/ib_umem_release() to modules")

>
> Could you please give more suggestion? Thanks.

Please, don't use top-posting for this mailing list.
It is really-really annoying.

>
> On Thu, Dec 29, 2016 at 10:17:56AM +0200, Leon Romanovsky wrote:
> > Date: Thu, 29 Dec 2016 10:17:56 +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 v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()
> > User-Agent: Mutt/1.7.2 (2016-11-26)
> > Message-ID: <20161229081756.GI26885@mtr-leonro.local>
> >
> > On Thu, Dec 29, 2016 at 04:27:28PM +0800, Kenneth Lee wrote:
> > > There are two bugfixes in this patch:
> > >
> > > 1. When the execution go to the ib_umem_odp_get() path, pid should be put
> > >    back.
> > > 2. When the memory allocation fail, the pid also should be put back before
> > >    exit.
> > >
> > > 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
> >
> > I don't see any changes except version in the title.
> > What about anything like this?
> > [PATCH v3] IB/umem: Release pid in error and ODP flows
> >
> > And Fixes line please, it will help to forward it to stable trees.
> >
> > Thanks
>
>
>
> --
> 			-Kenneth(Hisilicon)

[toc] | [prev] | [next] | [standalone]

#1548517

On Fri, Dec 30, 2016 at 08:55:10AM +0200, Leon Romanovsky wrote:
> Date: Fri, 30 Dec 2016 08:55:10 +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 v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()
> User-Agent: Mutt/1.7.2 (2016-11-26)
> Message-ID: <20161230065510.GL26885@mtr-leonro.local>
> 
> On Fri, Dec 30, 2016 at 12:50:11PM +0800, Kenneth Lee wrote:
> > Hi, Leon,
> >
> > 1. I do change the title except for the version number itself:) But my English
> > is quite bad, maybe the title is still quite stupid. I can update it according
> > to your advice.
> 
> Yes, please
> The main points are:
> 1. Remove "bugifix", it is not needed.
> 2. Use description in the title and not function names.
> 
> >
> > 2. I catched the bug by reading the final code, not by bisect-ing the old
> > commit. Do you means I should find out which commit introducing the bug? It will
> > not be easily to say which it is because it is a "missing bug", rather than a
> > "introduced bug". Indicate the commit may not help to remove a patch/commit from
> > the stable tree.
> 
> The fixes line won't cause for removal of commit, but to addition of
> yours on top of their code base.
> 
> git blame is your friend.
> 
> one fixes line is:
> Fixes: 8ada2c1c0c1d ("IB/core: Add support for on demand paging regions")
> 
> and the second line is !!!!! NOT !!!!!, you need to go deeper in the logs !!!!!!
> Fixes: f7c6a7b5d599 ("IB/uverbs: Export ib_umem_get()/ib_umem_release() to modules")
> 
> >
> > Could you please give more suggestion? Thanks.
> 
> Please, don't use top-posting for this mailing list.
> It is really-really annoying.
> 
> >
> > On Thu, Dec 29, 2016 at 10:17:56AM +0200, Leon Romanovsky wrote:
> > > Date: Thu, 29 Dec 2016 10:17:56 +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 v2] ib umem: bugfix: mixed put_pid()s in ib_umem_get()
> > > User-Agent: Mutt/1.7.2 (2016-11-26)
> > > Message-ID: <20161229081756.GI26885@mtr-leonro.local>
> > >
> > > On Thu, Dec 29, 2016 at 04:27:28PM +0800, Kenneth Lee wrote:
> > > > There are two bugfixes in this patch:
> > > >
> > > > 1. When the execution go to the ib_umem_odp_get() path, pid should be put
> > > >    back.
> > > > 2. When the memory allocation fail, the pid also should be put back before
> > > >    exit.
> > > >
> > > > 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
> > >
> > > I don't see any changes except version in the title.
> > > What about anything like this?
> > > [PATCH v3] IB/umem: Release pid in error and ODP flows
> > >
> > > And Fixes line please, it will help to forward it to stable trees.
> > >
> > > Thanks
> >
> >
> >
> > --
> > 			-Kenneth(Hisilicon)

Very helpful. Thank you. I will send the Patch v3 soon.

-- 
			-Kenneth(Hisilicon)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web