headers
Srinivas Eeda | 8 May 01:21
Picon
Favicon

ocfs2 discontiguous localalloc patches

Hi all,

can you please review following 3 patches that implement discontiguous
localalloc bitmap support for ocfs2 file system. This feature helps
applications that significantly fragment the filesystem.

These fixes needs changes to ocfs2 tools as well. I am sending those patches
for review separately.

A write up on this feature is available at
http://oss.oracle.com/osswiki/OCFS2/DesignDocs/DiscontiguousLocalAlloc.html

Thanks,
--Srini
Permalink | Reply | 10 comments |
headers
Dan Carpenter | 15 Feb 21:31
Picon
Favicon

Re: Ocfs2/move_extents: move/defrag extents within a certain range.

Hello Tristan Ye,

This is a semi-automatic email about new static checker warnings.

The patch 53069d4e7695: "Ocfs2/move_extents: move/defrag extents 
within a certain range." from May 25, 2011, leads to the following 
Smatch complaint:

fs/ocfs2/move_extents.c:980 ocfs2_move_extents()
	 warn: variable dereferenced before check 'inode' (see line 978)

fs/ocfs2/move_extents.c
   977		struct buffer_head *di_bh = NULL;
   978		struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
                                                   ^^^^^^^^^^^
Dereference.

   979	
   980		if (!inode)
                    ^^^^^^
Check.

   981			return -ENOENT;
   982	

regards,
dan carpenter
Permalink | Reply | 1 comment |
headers
Dan Carpenter | 13 Feb 14:50
Picon
Favicon

[patch] ocfs2: cleanup error handling in o2hb_alloc_hb_set()

If "ret" is NULL, then "hs" is also NULL, so there is no need to free
it.  config_group_init_type_name() can't fail if the name ("heartbeat"
in this case) is less than CONFIGFS_ITEM_NAME_LEN (20) characters long
so we can just remove this error handling code.

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

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index f3f2d95..0bca51b 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -2306,20 +2306,14 @@ static struct config_item_type o2hb_heartbeat_group_type = {
 struct config_group *o2hb_alloc_hb_set(void)
 {
 	struct o2hb_heartbeat_group *hs = NULL;
-	struct config_group *ret = NULL;

 	hs = kzalloc(sizeof(struct o2hb_heartbeat_group), GFP_KERNEL);
 	if (hs == NULL)
-		goto out;
+		return NULL;

 	config_group_init_type_name(&hs->hs_group, "heartbeat",
 				    &o2hb_heartbeat_group_type);
-
-	ret = &hs->hs_group;
-out:
-	if (ret == NULL)
-		kfree(hs);
-	return ret;
(Continue reading)

Permalink | Reply | 7 comments |
headers
Chang Limin | 13 Apr 05:32
Favicon

Maybe a null point bug in __ocfs2_change_file_space.

Hi,

 Version linux-3.3

In function

static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,

                        loff_t len)

{

…

return __ocfs2_change_file_space(NULL, inode, offset, cmd, &sr,           // file is NULL

                                        change_size);

}

 

In function

static int __ocfs2_change_file_space(struct file *file, struct inode *inode,

                                  loff_t f_pos, unsigned int cmd,

                                  struct ocfs2_space_resv *sr,

                                  int change_size)

{

if (file->f_flags & O_SYNC)                                                                                   // access file->f_flags result null pointer

           handle->h_sync = 1;

}

 

Changlimin

 

-------------------------------------------------------------------------------------------------------------------------------------
本邮件及其附件含有杭州华三通信技术有限公司的保密信息，仅限于发送给上面地址中列出
的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、
或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本
邮件！
This e-mail and its attachments contain confidential information from H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it! 
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel <at> oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Permalink | Reply | 0 comments |
headers
Chang Limin | 13 Apr 09:58
Favicon

Maybe a null point bug in __ocfs2_change_file_space.

Hi,

 Version linux-3.3

In function

static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,

                        loff_t len)

{

…

return __ocfs2_change_file_space(NULL, inode, offset, cmd, &sr,           // file is NULL

                                        change_size);

}

 

In function

static int __ocfs2_change_file_space(struct file *file, struct inode *inode,

                                  loff_t f_pos, unsigned int cmd,

                                  struct ocfs2_space_resv *sr,

                                  int change_size)

{

if (file->f_flags & O_SYNC)                                                                                   // access file->f_flags result null pointer

           handle->h_sync = 1;

}

 

Changlimin

-------------------------------------------------------------------------------------------------------------------------------------
本邮件及其附件含有杭州华三通信技术有限公司的保密信息，仅限于发送给上面地址中列出
的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、
或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本
邮件！
This e-mail and its attachments contain confidential information from H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it! 
_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel <at> oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel
Permalink | Reply | 1 comment |
headers
Sunil Mushran | 1 Mar 20:34
Picon
Favicon

[PATCH 1/9] ocfs2/cluster: Fix possible null pointer dereference

Patch fixes some possible null pointer dereferences that were detected by the
static code analyser, smatch.

Reported-by: Dan Carpenter <error27 <at> gmail.com>
Signed-off-by: Sunil Mushran <sunil.mushran <at> oracle.com>
---
 fs/ocfs2/cluster/tcp.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
index 044e7b5..bf9494d 100644
--- a/fs/ocfs2/cluster/tcp.c
+++ b/fs/ocfs2/cluster/tcp.c
@@ -546,8 +546,9 @@ static void o2net_set_nn_state(struct o2net_node *nn,
 	}

 	if (was_valid && !valid) {
-		printk(KERN_NOTICE "o2net: No longer connected to "
-		       SC_NODEF_FMT "\n", SC_NODEF_ARGS(old_sc));
+		if (old_sc)
+			printk(KERN_NOTICE "o2net: No longer connected to "
+			       SC_NODEF_FMT "\n", SC_NODEF_ARGS(old_sc));
 		o2net_complete_nodes_nsw(nn);
 	}

@@ -1700,13 +1701,12 @@ static void o2net_start_connect(struct work_struct *work)
 		ret = 0;

 out:
-	if (ret) {
+	if (ret && sc) {
 		printk(KERN_NOTICE "o2net: Connect attempt to " SC_NODEF_FMT
 		       " failed with errno %d\n", SC_NODEF_ARGS(sc), ret);
 		/* 0 err so that another will be queued and attempted
 		 * from set_nn_state */
-		if (sc)
-			o2net_ensure_shutdown(nn, sc, 0);
+		o2net_ensure_shutdown(nn, sc, 0);
 	}
 	if (sc)
 		sc_put(sc);
--

-- 
1.7.7.6
Permalink | Reply | 8 comments |
headers
xiaowei.hu | 21 Feb 07:12
Picon
Favicon

Race condition between OCFS2 downconvert thread and ocfs2 cluster lock.

I am trying to fix bug13611997,CT's machine run into BUG in ocfs2dc thread, BUG_ON(lockres->l_action !=
OCFS2_AST_CONVERT && lockres->l_action != OCFS2_AST_DOWNCONVERT); I analysized the vmcore , the
lockres->l_action = OCFS2_AST_ATTACH and l_flags=326(which means
OCFS2_LOCK_BUSY|OCFS2_LOCK_BLOCKED|OCFS2_LOCK_INITIALIZED|OCFS2_LOCK_QUEUED), after
compared with the code , this status could be only possible during ocfs2_cluster_lock,here is the race situation:

NodeA								NodeB
ocfs2_cluster_lock on a new lockres M
spin_lock_irqsave(&lockres->l_lock, flags);
gen = lockres_set_pending(lockres);
lockres->l_action = OCFS2_AST_ATTACH;
lockres_or_flags(lockres, OCFS2_LOCK_BUSY);
spin_unlock_irqrestore(&lockres->l_lock, flags);

ocfs2_dlm_lock() finished and returned.
**and lockres_clear_pending(lockres, gen, osb);
							request a lock on the same lockres M
							It's blocked by nodeA, and a ast proxy was send to A

bast queued and flushed,before the ast was queued
then the ocfs2dc was scheduled
there is a chance to execute this code path:
ocfs2_downconvert_thread()
ocfs2_downconvert_thread_do_work()
ocfs2_blocking_ast()
ocfs2_process_blocked_lock()
ocfs2_unblock_lock()
	spin_lock_irqsave(&lockres->l_lock, flags);
	if (lockres->l_flags & OCFS2_LOCK_BUSY)
	    ret = ocfs2_prepare_cancel_convert(osb, lockres);
		BUG_ON(lockres->l_action != OCFS2_AST_CONVERT &&
               	       lockres->l_action != OCFS2_AST_DOWNCONVERT);
		here trigger the BUG()

Solution:
One possible solution for this is to remove the lockres_clear_pending marked by 2 stars, and left this
clear work to the ast function.In this way could make sure the bast function wait for ast , let it clear
OCFS2_LOCK_BUSY and set OCFS2_LOCK_ATTACHED first, before enter downconvert process.
Permalink | Reply | 8 comments |
headers
Jan Kara | 10 Feb 10:50
Picon

[PATCH] ocfs2: Fix bogus error message from ocfs2_global_read_info

'status' variable in ocfs2_global_read_info() is always != 0 when leaving the
function because it happens to contain number of read bytes. Thus we always log
error message although everything is OK. Since all error cases properly call
mlog_errno() before jumping to out_err, there's no reason to call mlog_errno()
on exit at all. This is a fallout of c1e8d35e (conversion of mlog_exit()
calls).

Signed-off-by: Jan Kara <jack <at> suse.cz>
---
 fs/ocfs2/quota_global.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/quota_global.c b/fs/ocfs2/quota_global.c
index 92fcd57..0a86e30 100644
--- a/fs/ocfs2/quota_global.c
+++ b/fs/ocfs2/quota_global.c
@@ -399,8 +399,6 @@ int ocfs2_global_read_info(struct super_block *sb, int type)
 			      msecs_to_jiffies(oinfo->dqi_syncms));

 out_err:
-	if (status)
-		mlog_errno(status);
 	return status;
 out_unlock:
 	ocfs2_unlock_global_qf(oinfo, 0);
--

-- 
1.7.1
Permalink | Reply | 0 comments |
headers
Jeff Liu | 9 Feb 07:42
Picon
Favicon

[PATCH] ocfs2: for SEEK_DATA/SEEK_HOLE, return internal error unchanged if ocfs2_get_clusters_nocache() or ocfs2_inode_lock() call failed.

Hello,

Since ENXIO only means "offset beyond EOF" for SEEK_DATA/SEEK_HOLE,
Hence we should return the internal error unchanged if ocfs2_inode_lock() or
ocfs2_get_clusters_nocache() call failed rather than ENXIO.
Otherwise, it will confuse the user applications when they trying to understand the root cause.

Thanks Dave for pointing this out.

Thanks,
-Jeff

Cc: Dave Chinner <david <at> fromorbit.com>
Signed-off-by: Jie Liu <jeff.liu <at> oracle.com>

---
 fs/ocfs2/extent_map.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 2f5b92e..70b5863 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -923,8 +923,6 @@ out_unlock:

 	ocfs2_inode_unlock(inode, 0);
 out:
-	if (ret && ret != -ENXIO)
-		ret = -ENXIO;
 	return ret;
 }

--

-- 
1.7.9
Permalink | Reply | 1 comment |
headers
Srinivas Eeda | 31 Jan 08:16
Picon
Favicon

[PATCH 1/1] o2dlm: fix NULL pointer dereference in o2dlm_blocking_ast_wrapper

A tiny race between BAST and unlock message causes the NULL dereference.

A node sends an unlock request to master and receives a response. Before
processing the response it receives a BAST from the master. Since both requests
are processed by different threads it creates a race. While the BAST is being
processed, lock can get freed by unlock code.

This patch makes bast to return immediately if lock is found but unlock is
pending. The code should handle this race. We also have to fix master node to
skip sending BAST after receiving unlock message.

Below is the crash stack

BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
IP: [<ffffffffa015e023>] o2dlm_blocking_ast_wrapper+0xd/0x16
[<ffffffffa034e3db>] dlm_do_local_bast+0x8e/0x97 [ocfs2_dlm]
[<ffffffffa034f366>] dlm_proxy_ast_handler+0x838/0x87e [ocfs2_dlm]
[<ffffffffa0308abe>] o2net_process_message+0x395/0x5b8 [ocfs2_nodemanager]
[<ffffffffa030aac8>] o2net_rx_until_empty+0x762/0x90d [ocfs2_nodemanager]
[<ffffffff81071802>] worker_thread+0x14d/0x1ed

Signed-off-by: Srinivas Eeda <srinivas.eeda <at> oracle.com>
---
 fs/ocfs2/dlm/dlmast.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/ocfs2/dlm/dlmast.c b/fs/ocfs2/dlm/dlmast.c
index 3a3ed4b..1281e8a 100644
--- a/fs/ocfs2/dlm/dlmast.c
+++ b/fs/ocfs2/dlm/dlmast.c
@@ -386,8 +386,9 @@ int dlm_proxy_ast_handler(struct o2net_msg *msg, u32 len, void *data,
 		head = &res->granted;

 	list_for_each(iter, head) {
+		/* if lock is found but unlock is pending ignore the bast */
 		lock = list_entry (iter, struct dlm_lock, list);
-		if (lock->ml.cookie == cookie)
+		if ((lock->ml.cookie == cookie) && (!lock->unlock_pending))
 			goto do_ast;
 	}

--

-- 
1.5.4.3
Permalink | Reply | 0 comments |
headers
Srinivas Eeda | 31 Jan 06:51
Picon
Favicon

[PATCH 1/1] ocfs2: use spinlock irqsave for downconvert lock.patch

When ocfs2dc thread holds dc_task_lock spinlock and receives soft IRQ it
deadlock itself trying to get same spinlock in ocfs2_wake_downconvert_thread.
Below is the stack snippet.

The patch disables interrupts when acquiring dc_task_lock spinlock.

	ocfs2_wake_downconvert_thread
	ocfs2_rw_unlock
	ocfs2_dio_end_io
	dio_complete
	.....
	bio_endio
	req_bio_endio
	....
	scsi_io_completion
	blk_done_softirq
	__do_softirq
	do_softirq
	irq_exit
	do_IRQ
	ocfs2_downconvert_thread
	[kthread]

Signed-off-by: Srinivas Eeda <srinivas.eeda <at> oracle.com>
---
 fs/ocfs2/dlmglue.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 81a4cd2..67af5db 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -3932,6 +3932,8 @@ unqueue:
 static void ocfs2_schedule_blocked_lock(struct ocfs2_super *osb,
 					struct ocfs2_lock_res *lockres)
 {
+	unsigned long flags;
+
 	assert_spin_locked(&lockres->l_lock);

 	if (lockres->l_flags & OCFS2_LOCK_FREEING) {
@@ -3945,21 +3947,22 @@ static void ocfs2_schedule_blocked_lock(struct ocfs2_super *osb,

 	lockres_or_flags(lockres, OCFS2_LOCK_QUEUED);

-	spin_lock(&osb->dc_task_lock);
+	spin_lock_irqsave(&osb->dc_task_lock, flags);
 	if (list_empty(&lockres->l_blocked_list)) {
 		list_add_tail(&lockres->l_blocked_list,
 			      &osb->blocked_lock_list);
 		osb->blocked_lock_count++;
 	}
-	spin_unlock(&osb->dc_task_lock);
+	spin_unlock_irqrestore(&osb->dc_task_lock, flags);
 }

 static void ocfs2_downconvert_thread_do_work(struct ocfs2_super *osb)
 {
 	unsigned long processed;
+	unsigned long flags;
 	struct ocfs2_lock_res *lockres;

-	spin_lock(&osb->dc_task_lock);
+	spin_lock_irqsave(&osb->dc_task_lock, flags);
 	/* grab this early so we know to try again if a state change and
 	 * wake happens part-way through our work  */
 	osb->dc_work_sequence = osb->dc_wake_sequence;
@@ -3972,38 +3975,40 @@ static void ocfs2_downconvert_thread_do_work(struct ocfs2_super *osb)
 				     struct ocfs2_lock_res, l_blocked_list);
 		list_del_init(&lockres->l_blocked_list);
 		osb->blocked_lock_count--;
-		spin_unlock(&osb->dc_task_lock);
+		spin_unlock_irqrestore(&osb->dc_task_lock, flags);

 		BUG_ON(!processed);
 		processed--;

 		ocfs2_process_blocked_lock(osb, lockres);

-		spin_lock(&osb->dc_task_lock);
+		spin_lock_irqsave(&osb->dc_task_lock, flags);
 	}
-	spin_unlock(&osb->dc_task_lock);
+	spin_unlock_irqrestore(&osb->dc_task_lock, flags);
 }

 static int ocfs2_downconvert_thread_lists_empty(struct ocfs2_super *osb)
 {
 	int empty = 0;
+	unsigned long flags;

-	spin_lock(&osb->dc_task_lock);
+	spin_lock_irqsave(&osb->dc_task_lock, flags);
 	if (list_empty(&osb->blocked_lock_list))
 		empty = 1;

-	spin_unlock(&osb->dc_task_lock);
+	spin_unlock_irqrestore(&osb->dc_task_lock, flags);
 	return empty;
 }

 static int ocfs2_downconvert_thread_should_wake(struct ocfs2_super *osb)
 {
 	int should_wake = 0;
+	unsigned long flags;

-	spin_lock(&osb->dc_task_lock);
+	spin_lock_irqsave(&osb->dc_task_lock, flags);
 	if (osb->dc_work_sequence != osb->dc_wake_sequence)
 		should_wake = 1;
-	spin_unlock(&osb->dc_task_lock);
+	spin_unlock_irqrestore(&osb->dc_task_lock, flags);

 	return should_wake;
 }
@@ -4033,10 +4038,12 @@ static int ocfs2_downconvert_thread(void *arg)

 void ocfs2_wake_downconvert_thread(struct ocfs2_super *osb)
 {
-	spin_lock(&osb->dc_task_lock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&osb->dc_task_lock, flags);
 	/* make sure the voting thread gets a swipe at whatever changes
 	 * the caller may have made to the voting state */
 	osb->dc_wake_sequence++;
-	spin_unlock(&osb->dc_task_lock);
+	spin_unlock_irqrestore(&osb->dc_task_lock, flags);
 	wake_up(&osb->dc_event);
 }
--

-- 
1.5.4.3
Permalink | Reply | 1 comment |

Gmane