[Ocfs2-devel] Orphan Scan - v3

[Ocfs2-devel] Orphan Scan - v3

[Sunil Mushran]

Patch 5 has been respun with the recommended change... remove the ex
argument in ocfs2_orphan_scan_lock() and ocfs2_orphan_scan_unlock().

Sunil

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Sunil Mushran]

Local and Hard-RO mounts do not need orphan scanning.

Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>
---
 fs/ocfs2/dlmglue.c |   26 ++++++++++++++++----------
 fs/ocfs2/dlmglue.h |    4 ++--
 fs/ocfs2/journal.c |   30 +++++++++++++++++-------------
 fs/ocfs2/journal.h |    2 +-
 fs/ocfs2/super.c   |   32 +++++++++++++++-----------------
 5 files changed, 51 insertions(+), 43 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 667701e..8036800 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2365,15 +2365,20 @@ void ocfs2_inode_unlock(struct inode *inode,
 	mlog_exit_void();
 }
 
-int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno, int ex)
+int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno)
 {
 	struct ocfs2_lock_res *lockres;
 	struct ocfs2_orphan_scan_lvb *lvb;
-	int level = ex ? DLM_LOCK_EX : DLM_LOCK_PR;
 	int status = 0;
 
+	if (ocfs2_is_hard_readonly(osb))
+		return -EROFS;
+
+	if (ocfs2_mount_local(osb))
+		return 0;
+
 	lockres = &osb->osb_orphan_scan.os_lockres;
-	status = ocfs2_cluster_lock(osb, lockres, level, 0, 0);
+	status = ocfs2_cluster_lock(osb, lockres, DLM_LOCK_EX, 0, 0);
 	if (status < 0)
 		return status;
 
@@ -2386,17 +2391,18 @@ int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno, int ex)
 	return status;
 }
 
-void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno, int ex)
+void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno)
 {
 	struct ocfs2_lock_res *lockres;
 	struct ocfs2_orphan_scan_lvb *lvb;
-	int level = ex ? DLM_LOCK_EX : DLM_LOCK_PR;
 
-	lockres = &osb->osb_orphan_scan.os_lockres;
-	lvb = ocfs2_dlm_lvb(&lockres->l_lksb);
-	lvb->lvb_version = OCFS2_ORPHAN_LVB_VERSION;
-	lvb->lvb_os_seqno = cpu_to_be32(seqno);
-	ocfs2_cluster_unlock(osb, lockres, level);
+	if (!ocfs2_is_hard_readonly(osb) && !ocfs2_mount_local(osb)) {
+		lockres = &osb->osb_orphan_scan.os_lockres;
+		lvb = ocfs2_dlm_lvb(&lockres->l_lksb);
+		lvb->lvb_version = OCFS2_ORPHAN_LVB_VERSION;
+		lvb->lvb_os_seqno = cpu_to_be32(seqno);
+		ocfs2_cluster_unlock(osb, lockres, DLM_LOCK_EX);
+	}
 }
 
 int ocfs2_super_lock(struct ocfs2_super *osb,
diff --git a/fs/ocfs2/dlmglue.h b/fs/ocfs2/dlmglue.h
index 31b90d7..30f6831 100644
--- a/fs/ocfs2/dlmglue.h
+++ b/fs/ocfs2/dlmglue.h
@@ -121,8 +121,8 @@ int ocfs2_super_lock(struct ocfs2_super *osb,
 		     int ex);
 void ocfs2_super_unlock(struct ocfs2_super *osb,
 			int ex);
-int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno, int ex);
-void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno, int ex);
+int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno);
+void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno);
 
 int ocfs2_rename_lock(struct ocfs2_super *osb);
 void ocfs2_rename_unlock(struct ocfs2_super *osb);
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 0b2c27a..f033760 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1883,7 +1883,7 @@ void ocfs2_queue_orphan_scan(struct ocfs2_super *osb)
 	if (atomic_read(&os->os_state) == ORPHAN_SCAN_INACTIVE)
 		goto out;
 
-	status = ocfs2_orphan_scan_lock(osb, &seqno, DLM_LOCK_EX);
+	status = ocfs2_orphan_scan_lock(osb, &seqno);
 	if (status < 0) {
 		if (status != -EAGAIN)
 			mlog_errno(status);
@@ -1910,7 +1910,7 @@ void ocfs2_queue_orphan_scan(struct ocfs2_super *osb)
 	os->os_count++;
 	os->os_scantime = CURRENT_TIME;
 unlock:
-	ocfs2_orphan_scan_unlock(osb, seqno, DLM_LOCK_EX);
+	ocfs2_orphan_scan_unlock(osb, seqno);
 out:
 	return;
 }
@@ -1938,29 +1938,33 @@ void ocfs2_orphan_scan_stop(struct ocfs2_super *osb)
 	struct ocfs2_orphan_scan *os;
 
 	os = &osb->osb_orphan_scan;
-	atomic_set(&os->os_state, ORPHAN_SCAN_INACTIVE);
-	mutex_lock(&os->os_lock);
-	cancel_delayed_work(&os->os_orphan_scan_work);
-	mutex_unlock(&os->os_lock);
+	if (atomic_read(&os->os_state) == ORPHAN_SCAN_ACTIVE) {
+		atomic_set(&os->os_state, ORPHAN_SCAN_INACTIVE);
+		mutex_lock(&os->os_lock);
+		cancel_delayed_work(&os->os_orphan_scan_work);
+		mutex_unlock(&os->os_lock);
+	}
 }
 
-int ocfs2_orphan_scan_init(struct ocfs2_super *osb)
+void ocfs2_orphan_scan_init(struct ocfs2_super *osb)
 {
 	struct ocfs2_orphan_scan *os;
 
 	os = &osb->osb_orphan_scan;
-	atomic_set(&os->os_state, ORPHAN_SCAN_ACTIVE);
 	os->os_osb = osb;
 	os->os_count = 0;
 	os->os_seqno = 0;
 	os->os_scantime = CURRENT_TIME;
 	mutex_init(&os->os_lock);
+	INIT_DELAYED_WORK(&os->os_orphan_scan_work, ocfs2_orphan_scan_work);
 
-	INIT_DELAYED_WORK(&os->os_orphan_scan_work,
-			  ocfs2_orphan_scan_work);
-	schedule_delayed_work(&os->os_orphan_scan_work,
-			      ocfs2_orphan_scan_timeout());
-	return 0;
+	if (ocfs2_is_hard_readonly(osb) || ocfs2_mount_local(osb))
+		atomic_set(&os->os_state, ORPHAN_SCAN_INACTIVE);
+	else {
+		atomic_set(&os->os_state, ORPHAN_SCAN_ACTIVE);
+		schedule_delayed_work(&os->os_orphan_scan_work,
+				      ocfs2_orphan_scan_timeout());
+	}
 }
 
 struct ocfs2_orphan_filldir_priv {
diff --git a/fs/ocfs2/journal.h b/fs/ocfs2/journal.h
index 61045ee..5432c7f 100644
--- a/fs/ocfs2/journal.h
+++ b/fs/ocfs2/journal.h
@@ -144,7 +144,7 @@ static inline void ocfs2_inode_set_new(struct ocfs2_super *osb,
 }
 
 /* Exported only for the journal struct init code in super.c. Do not call. */
-int ocfs2_orphan_scan_init(struct ocfs2_super *osb);
+void ocfs2_orphan_scan_init(struct ocfs2_super *osb);
 void ocfs2_orphan_scan_stop(struct ocfs2_super *osb);
 void ocfs2_orphan_scan_exit(struct ocfs2_super *osb);
 
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 3e8a68b..0746e1a 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -205,11 +205,10 @@ static const match_table_t tokens = {
 #ifdef CONFIG_DEBUG_FS
 static int ocfs2_osb_dump(struct ocfs2_super *osb, char *buf, int len)
 {
-	int out = 0;
-	int i;
 	struct ocfs2_cluster_connection *cconn = osb->cconn;
 	struct ocfs2_recovery_map *rm = osb->recovery_map;
-	struct ocfs2_orphan_scan *os;
+	struct ocfs2_orphan_scan *os = &osb->osb_orphan_scan;
+	int i, out = 0;
 
 	out += snprintf(buf + out, len - out,
 			"%10s => Id: %-s  Uuid: %-s  Gen: 0x%X  Label: %-s\n",
@@ -305,6 +304,16 @@ static int ocfs2_osb_dump(struct ocfs2_super *osb, char *buf, int len)
 			atomic_read(&osb->s_num_inodes_stolen));
 	spin_unlock(&osb->osb_lock);
 
+	out += snprintf(buf + out, len - out, "OrphanScan => ");
+	out += snprintf(buf + out, len - out, "Local: %u  Global: %u ",
+			os->os_count, os->os_seqno);
+	out += snprintf(buf + out, len - out, " Last Scan: ");
+	if (atomic_read(&os->os_state) == ORPHAN_SCAN_INACTIVE)
+		out += snprintf(buf + out, len - out, "Disabled\n");
+	else
+		out += snprintf(buf + out, len - out, "%lu seconds ago\n",
+				(get_seconds() - os->os_scantime.tv_sec));
+
 	out += snprintf(buf + out, len - out, "%10s => %3s  %10s\n",
 			"Slots", "Num", "RecoGen");
 	for (i = 0; i < osb->max_slots; ++i) {
@@ -315,13 +324,6 @@ static int ocfs2_osb_dump(struct ocfs2_super *osb, char *buf, int len)
 				i, osb->slot_recovery_generations[i]);
 	}
 
-	os = &osb->osb_orphan_scan;
-	out += snprintf(buf + out, len - out, "Orphan Scan=> ");
-	out += snprintf(buf + out, len - out, "Local: %u  Global: %u ",
-			os->os_count, os->os_seqno);
-	out += snprintf(buf + out, len - out, " Last Scan: %lu seconds ago\n",
-			(get_seconds() - os->os_scantime.tv_sec));
-
 	return out;
 }
 
@@ -1179,6 +1181,9 @@ static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)
 	atomic_set(&osb->vol_state, VOLUME_MOUNTED_QUOTAS);
 	wake_up(&osb->osb_mount_event);
 
+	/* Start this when the mount is almost sure of being successful */
+	ocfs2_orphan_scan_init(osb);
+
 	mlog_exit(status);
 	return status;
 
@@ -1983,13 +1988,6 @@ static int ocfs2_initialize_super(struct super_block *sb,
 		goto bail;
 	}
 
-	status = ocfs2_orphan_scan_init(osb);
-	if (status) {
-		mlog(ML_ERROR, "Unable to initialize delayed orphan scan\n");
-		mlog_errno(status);
-		goto bail;
-	}
-
 	init_waitqueue_head(&osb->checkpoint_event);
 	atomic_set(&osb->needs_checkpoint, 0);
 
-- 
1.6.0.4

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Joel Becker]

On Mon, Jun 22, 2009 at 11:40:07AM -0700, Sunil Mushran wrote:
> Local and Hard-RO mounts do not need orphan scanning.

	I just realized that we hold the os->os_lock around everything
we do here.  The orphan scan state doesn't need to be atomic_t.

Joel


-- 

"I don't know anything about music. In my line you don't have
 to."
        - Elvis Presley

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Sunil Mushran]

Joel Becker wrote:
> On Mon, Jun 22, 2009 at 11:40:07AM -0700, Sunil Mushran wrote:
>> Local and Hard-RO mounts do not need orphan scanning.
> 	I just realized that we hold the os->os_lock around everything
> we do here.  The orphan scan state doesn't need to be atomic_t.

No we don't hold it across all ops. In ocfs2_orphan_scan_stop(), we set
it inactive before taking the mutex. This allows ocfs2_queue_orphan_scan()
to bail out without queuing the tasks. The idea is to prevent umount 
slowdown
- as much as possible.

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Joel Becker]

On Mon, Jun 22, 2009 at 01:26:40PM -0700, Sunil Mushran wrote:
> Joel Becker wrote:
> > On Mon, Jun 22, 2009 at 11:40:07AM -0700, Sunil Mushran wrote:
> >> Local and Hard-RO mounts do not need orphan scanning.
> > 	I just realized that we hold the os->os_lock around everything
> > we do here.  The orphan scan state doesn't need to be atomic_t.
> 
> No we don't hold it across all ops. In ocfs2_orphan_scan_stop(), we set
> it inactive before taking the mutex. This allows ocfs2_queue_orphan_scan()
> to bail out without queuing the tasks. The idea is to prevent umount 
> slowdown
> - as much as possible.

	It still strikes me as over-optimized, but it was a criticism
better leveled at the initial patch.  This patch is about local and
hard-readonly, and it's ready to go.

Joel

-- 

"But then she looks me in the eye
 And says, 'We're going to last forever,'
 And man you know I can't begin to doubt it.
 Cause it just feels so good and so free and so right,
 I know we ain't never going to change our minds about it, Hey!
 Here comes my girl."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Jeff Mahoney]

Joel Becker wrote:
> On Mon, Jun 22, 2009 at 01:26:40PM -0700, Sunil Mushran wrote:
>> Joel Becker wrote:
>>> On Mon, Jun 22, 2009 at 11:40:07AM -0700, Sunil Mushran wrote:
>>>> Local and Hard-RO mounts do not need orphan scanning.
>>> 	I just realized that we hold the os->os_lock around everything
>>> we do here.  The orphan scan state doesn't need to be atomic_t.
>> No we don't hold it across all ops. In ocfs2_orphan_scan_stop(), we set
>> it inactive before taking the mutex. This allows ocfs2_queue_orphan_scan()
>> to bail out without queuing the tasks. The idea is to prevent umount 
>> slowdown
>> - as much as possible.
> 
> 	It still strikes me as over-optimized, but it was a criticism
> better leveled at the initial patch.  This patch is about local and
> hard-readonly, and it's ready to go.

Hi guys -

There's a problem with this patch that I ran into while testing
2.6.31-rc2. ocfs2_orphan_scan_stop expects that ocfs2_orphan_scan_init
was called. If the mount fails for any reason, ocfs2_dismount_volume
calls ocfs2_orphan_scan_stop. The thing is that ocfs2_dismount_volume
will never get called in that path after ocfs2_orphan_scan_init is
called, so we oops when calling cancel_delayed_work on an uninitialized
work queue.

The attached patch fixes it up.

-Jeff

-- 
Jeff Mahoney
SUSE Labs
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ocfs2-orphan-scan-fixup
Url: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20090707/92aae00b/attachment.pl 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 257 bytes
Desc: OpenPGP digital signature
Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20090707/92aae00b/attachment.bin 

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Sunil Mushran]

Jeff Mahoney wrote:
> There's a problem with this patch that I ran into while testing
> 2.6.31-rc2. ocfs2_orphan_scan_stop expects that ocfs2_orphan_scan_init
> was called. If the mount fails for any reason, ocfs2_dismount_volume
> calls ocfs2_orphan_scan_stop. The thing is that ocfs2_dismount_volume
> will never get called in that path after ocfs2_orphan_scan_init is
> called, so we oops when calling cancel_delayed_work on an uninitialized
> work queue.
>
> The attached patch fixes it up.
>
> -Jeff

Jeff, Thanks. For some reason, my mounts don't fail. ;)

My only quibble with the patch would be that ocfs2_orphan_scan_init()
should be called before ocfs2_recovery_init(). Else we'll encounter the
same problem if say ocfs2_recovery_init() fails.

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Jeff Mahoney]

Sunil Mushran wrote:
> Jeff Mahoney wrote:
>> There's a problem with this patch that I ran into while testing
>> 2.6.31-rc2. ocfs2_orphan_scan_stop expects that ocfs2_orphan_scan_init
>> was called. If the mount fails for any reason, ocfs2_dismount_volume
>> calls ocfs2_orphan_scan_stop. The thing is that ocfs2_dismount_volume
>> will never get called in that path after ocfs2_orphan_scan_init is
>> called, so we oops when calling cancel_delayed_work on an uninitialized
>> work queue.
>>
>> The attached patch fixes it up.
>>
>> -Jeff
> 
> Jeff, Thanks. For some reason, my mounts don't fail. ;)

Yeah, mine don't usually either, but I installed our desktop product on
one of my development nodes and gnome tries to mount everything. I have
an OCFS2 file system on a disk but the cluster isn't set up.

> My only quibble with the patch would be that ocfs2_orphan_scan_init()
> should be called before ocfs2_recovery_init(). Else we'll encounter the
> same problem if say ocfs2_recovery_init() fails.

Yup, you're right. Fixed version attached.

-Jeff

--
Jeff Mahoney
SuSE Labs
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ocfs2-orphan-scan-fixup
Url: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20090707/92329fdb/attachment.pl 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 257 bytes
Desc: OpenPGP digital signature
Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20090707/92329fdb/attachment.bin 

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Sunil Mushran]

Jeff Mahoney wrote:
> Yup, you're right. Fixed version attached.

Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>

[Ocfs2-devel] [PATCH 5/5] ocfs2: Disable orphan scanning for local and hard-ro mounts

[Joel Becker]

On Tue, Jul 07, 2009 at 05:22:12PM -0400, Jeff Mahoney wrote:
> Subject: ocfs2: Fixup orphan scan cleanup after failed mount
> 
> 
>  If the mount fails for any reason, ocfs2_dismount_volume calls
>  ocfs2_orphan_scan_stop. It requires that ocfs2_orphan_scan_init
>  be called to setup the mutex and work queues, but that doesn't
>  happen if the mount has failed and we oops accessing an uninitialized
>  work queue.
> 
>  This patch splits the init and startup of the orphan scan, eliminating
>  the oops.

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

Joel

-- 

"Conservative, n.  A statesman who is enamoured of existing evils,
 as distinguished from the Liberal, who wishes to replace them
 with others."
	- Ambrose Bierce, The Devil's Dictionary

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127