[patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Dan Carpenter]

My static checker complains that this is called with a spin_lock held
in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
we have not received any bug reports about this is that recovery is not
a common operation.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 005261c..33ecbe0 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -2020,7 +2020,7 @@ int dlm_dispatch_assert_master(struct dlm_ctxt *dlm,
 			       int ignore_higher, u8 request_from, u32 flags)
 {
 	struct dlm_work_item *item;
-	item = kzalloc(sizeof(*item), GFP_NOFS);
+	item = kzalloc(sizeof(*item), GFP_ATOMIC);
 	if (!item)
 		return -ENOMEM;
 

[Ocfs2-devel] [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Dan Carpenter]

My static checker complains that this is called with a spin_lock held
in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
we have not received any bug reports about this is that recovery is not
a common operation.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 005261c..33ecbe0 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -2020,7 +2020,7 @@ int dlm_dispatch_assert_master(struct dlm_ctxt *dlm,
 			       int ignore_higher, u8 request_from, u32 flags)
 {
 	struct dlm_work_item *item;
-	item = kzalloc(sizeof(*item), GFP_NOFS);
+	item = kzalloc(sizeof(*item), GFP_ATOMIC);
 	if (!item)
 		return -ENOMEM;
 

Re: [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Mark Fasheh]

On Thu, Jul 26, 2012 at 04:05:05PM +0300, Dan Carpenter wrote:
> My static checker complains that this is called with a spin_lock held
> in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
> we have not received any bug reports about this is that recovery is not
> a common operation.

Looks reasonable to me. Sunil, we might just want to have that dlm_work_item
allocated by the caller, yes?

Acked-by: Mark Fasheh <mfasheh@suse.de>
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Mark Fasheh]

On Thu, Jul 26, 2012 at 04:05:05PM +0300, Dan Carpenter wrote:
> My static checker complains that this is called with a spin_lock held
> in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
> we have not received any bug reports about this is that recovery is not
> a common operation.

Looks reasonable to me. Sunil, we might just want to have that dlm_work_item
allocated by the caller, yes?

Acked-by: Mark Fasheh <mfasheh@suse.de>
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Sunil Mushran]

On Fri, Jul 27, 2012 at 1:32 PM, Mark Fasheh <mfasheh@suse.de> wrote:

> On Thu, Jul 26, 2012 at 04:05:05PM +0300, Dan Carpenter wrote:
> > My static checker complains that this is called with a spin_lock held
> > in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
> > we have not received any bug reports about this is that recovery is not
> > a common operation.
>
> Looks reasonable to me. Sunil, we might just want to have that
> dlm_work_item
> allocated by the caller, yes?
>


My static checker complains that this is called with a spin_lock held
in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
we have not received any bug reports about this is that recovery is not
a common operation.

Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
index 005261c..33ecbe0 100644
--- a/fs/ocfs2/dlm/dlmmaster.c
+++ b/fs/ocfs2/dlm/dlmmaster.c
@@ -2020,7 +2020,7 @@ int dlm_dispatch_assert_master(struct dlm_ctxt *dlm,
 			       int ignore_higher, u8 request_from, u32 flags)
 {
 	struct dlm_work_item *item;
-	item = kzalloc(sizeof(*item), GFP_NOFS);
+	item = kzalloc(sizeof(*item), GFP_ATOMIC);
 	if (!item)
 		return -ENOMEM;

--




I don't think the caller needs to hold the lockres spinlock. But this will
do too.

Yes, this is rare because this gets triggered only if a node that is
migrating
ownership (to or from) dies before migration completes. This part of the
code
is any case shaky as best as it is hard to test all cases.

Sunil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20120730/26e1b515/attachment.html 

Re: [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Joel Becker]

On Thu, Jul 26, 2012 at 04:05:05PM +0300, Dan Carpenter wrote:
> My static checker complains that this is called with a spin_lock held
> in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
> we have not received any bug reports about this is that recovery is not
> a common operation.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

This patch is now part of the fixes branch of ocfs2.git.

Joel

> 
> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> index 005261c..33ecbe0 100644
> --- a/fs/ocfs2/dlm/dlmmaster.c
> +++ b/fs/ocfs2/dlm/dlmmaster.c
> @@ -2020,7 +2020,7 @@ int dlm_dispatch_assert_master(struct dlm_ctxt *dlm,
>  			       int ignore_higher, u8 request_from, u32 flags)
>  {
>  	struct dlm_work_item *item;
> -	item = kzalloc(sizeof(*item), GFP_NOFS);
> +	item = kzalloc(sizeof(*item), GFP_ATOMIC);
>  	if (!item)
>  		return -ENOMEM;
>  

-- 

"We will have to repent in this generation not merely for the
 vitriolic words and actions of the bad people, but for the 
 appalling silence of the good people."
	- Rev. Dr. Martin Luther King, Jr.

			http://www.jlbec.org/
			jlbec@evilplan.org

[Ocfs2-devel] [patch] ocfs2/dlm: use GFP_ATOMIC inside a spin_lock

[Joel Becker]

On Thu, Jul 26, 2012 at 04:05:05PM +0300, Dan Carpenter wrote:
> My static checker complains that this is called with a spin_lock held
> in dlm_master_requery_handler() from dlmrecovery.c.  Probably the reason
> we have not received any bug reports about this is that recovery is not
> a common operation.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

This patch is now part of the fixes branch of ocfs2.git.

Joel

> 
> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c
> index 005261c..33ecbe0 100644
> --- a/fs/ocfs2/dlm/dlmmaster.c
> +++ b/fs/ocfs2/dlm/dlmmaster.c
> @@ -2020,7 +2020,7 @@ int dlm_dispatch_assert_master(struct dlm_ctxt *dlm,
>  			       int ignore_higher, u8 request_from, u32 flags)
>  {
>  	struct dlm_work_item *item;
> -	item = kzalloc(sizeof(*item), GFP_NOFS);
> +	item = kzalloc(sizeof(*item), GFP_ATOMIC);
>  	if (!item)
>  		return -ENOMEM;
>  

-- 

"We will have to repent in this generation not merely for the
 vitriolic words and actions of the bad people, but for the 
 appalling silence of the good people."
	- Rev. Dr. Martin Luther King, Jr.

			http://www.jlbec.org/
			jlbec at evilplan.org