[Ocfs2-devel] Orphan scan fixes - V2

[Ocfs2-devel] Orphan scan fixes - V2

[Sunil Mushran]

Implemented the suggestions.

Also, moved the ocfs2_orphan_scan_init() to the end of ocfs2_fill_super()
instead of calling it earlier in ocfs2_initialize_super().

Sunil

[Ocfs2-devel] [PATCH 3/5] ocfs2: Stop orphan scan as early as possible during umount

[Sunil Mushran]

Currently if the orphan scan fires a tick before the user issues the umount,
the umount will wait for the queued orphan scan tasks to complete.

This patch makes the umount stop the orphan scan as early as possible so as
to reduce the probability of the queued tasks slowing down the umount.

Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>
---
 fs/ocfs2/journal.c |   14 ++++++++++++--
 fs/ocfs2/ocfs2.h   |    6 ++++++
 fs/ocfs2/super.c   |    5 +++--
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 4a3b9e6..70215a2 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1880,6 +1880,9 @@ void ocfs2_queue_orphan_scan(struct ocfs2_super *osb)
 
 	os = &osb->osb_orphan_scan;
 
+	if (atomic_read(&os->os_state) == ORPHAN_SCAN_INACTIVE)
+		goto out;
+
 	status = ocfs2_orphan_scan_lock(osb, &seqno, DLM_LOCK_EX);
 	if (status < 0) {
 		if (status != -EAGAIN)
@@ -1887,6 +1890,10 @@ void ocfs2_queue_orphan_scan(struct ocfs2_super *osb)
 		goto out;
 	}
 
+	/* Do no queue the tasks if the volume is being umounted */
+	if (atomic_read(&os->os_state) == ORPHAN_SCAN_INACTIVE)
+		goto unlock;
+
 	if (os->os_seqno != seqno) {
 		os->os_seqno = seqno;
 		goto unlock;
@@ -1920,8 +1927,9 @@ void ocfs2_orphan_scan_work(struct work_struct *work)
 
 	mutex_lock(&os->os_lock);
 	ocfs2_queue_orphan_scan(osb);
-	schedule_delayed_work(&os->os_orphan_scan_work,
-			      ocfs2_orphan_scan_timeout());
+	if (atomic_read(&os->os_state) == ORPHAN_SCAN_ACTIVE)
+		schedule_delayed_work(&os->os_orphan_scan_work,
+				      ocfs2_orphan_scan_timeout());
 	mutex_unlock(&os->os_lock);
 }
 
@@ -1930,6 +1938,7 @@ 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);
@@ -1940,6 +1949,7 @@ int 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_scantime = CURRENT_TIME;
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index 18c1d9e..60e8950 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -154,6 +154,11 @@ struct ocfs2_lock_res {
 #endif
 };
 
+enum ocfs2_orphan_scan_state {
+	ORPHAN_SCAN_ACTIVE,
+	ORPHAN_SCAN_INACTIVE
+};
+
 struct ocfs2_orphan_scan {
 	struct mutex 		os_lock;
 	struct ocfs2_super 	*os_osb;
@@ -162,6 +167,7 @@ struct ocfs2_orphan_scan {
 	struct timespec		os_scantime;  /* time this node ran the scan */
 	u32			os_count;      /* tracks node specific scans */
 	u32  			os_seqno;       /* tracks cluster wide scans */
+	atomic_t		os_state;              /* ACTIVE or INACTIVE */
 };
 
 struct ocfs2_dlm_debug {
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index d64739b..3e8a68b 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1814,14 +1814,15 @@ static void ocfs2_dismount_volume(struct super_block *sb, int mnt_err)
 
 	debugfs_remove(osb->osb_ctxt);
 
+	/* Orphan scan should be stopped as early as possible */
+	ocfs2_orphan_scan_stop(osb);
+
 	ocfs2_disable_quotas(osb);
 
 	ocfs2_shutdown_local_alloc(osb);
 
 	ocfs2_truncate_log_shutdown(osb);
 
-	ocfs2_orphan_scan_stop(osb);
-
 	/* This will disable recovery and flush any recovery work. */
 	ocfs2_recovery_exit(osb);
 
-- 
1.6.0.4

[Ocfs2-devel] [PATCH 4/5] ocfs2: Do not initialize lvb in ocfs2_orphan_scan_lock_res_init()

[Sunil Mushran]

No need to call ocfs2_dlm_lvb() in ocfs2_orphan_scan_lock_res_init().

Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>
---
 fs/ocfs2/dlmglue.c |    7 +++----
 fs/ocfs2/journal.c |    1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 6cdeaa7..667701e 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -644,14 +644,10 @@ static void ocfs2_nfs_sync_lock_res_init(struct ocfs2_lock_res *res,
 static void ocfs2_orphan_scan_lock_res_init(struct ocfs2_lock_res *res,
 					    struct ocfs2_super *osb)
 {
-	struct ocfs2_orphan_scan_lvb *lvb;
-
 	ocfs2_lock_res_init_once(res);
 	ocfs2_build_lock_name(OCFS2_LOCK_TYPE_ORPHAN_SCAN, 0, 0, res->l_name);
 	ocfs2_lock_res_init_common(osb, res, OCFS2_LOCK_TYPE_ORPHAN_SCAN,
 				   &ocfs2_orphan_scan_lops, osb);
-	lvb = ocfs2_dlm_lvb(&res->l_lksb);
-	lvb->lvb_version = OCFS2_ORPHAN_LVB_VERSION;
 }
 
 void ocfs2_file_lock_res_init(struct ocfs2_lock_res *lockres,
@@ -2384,6 +2380,9 @@ int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno, int ex)
 	lvb = ocfs2_dlm_lvb(&lockres->l_lksb);
 	if (lvb->lvb_version == OCFS2_ORPHAN_LVB_VERSION)
 		*seqno = be32_to_cpu(lvb->lvb_os_seqno);
+	else
+		*seqno = osb->osb_orphan_scan.os_seqno + 1;
+ 
 	return status;
 }
 
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 70215a2..0b2c27a 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -1952,6 +1952,7 @@ int ocfs2_orphan_scan_init(struct ocfs2_super *osb)
 	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);
 
-- 
1.6.0.4

[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 |   18 +++++++++++++-----
 fs/ocfs2/journal.c |   26 +++++++++++++++-----------
 fs/ocfs2/journal.h |    2 +-
 fs/ocfs2/super.c   |   32 +++++++++++++++-----------------
 4 files changed, 44 insertions(+), 34 deletions(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 667701e..a9c989e 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -2372,6 +2372,12 @@ int ocfs2_orphan_scan_lock(struct ocfs2_super *osb, u32 *seqno, int ex)
 	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);
 	if (status < 0)
@@ -2392,11 +2398,13 @@ void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno, int ex)
 	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, level);
+	}
 }
 
 int ocfs2_super_lock(struct ocfs2_super *osb,
diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c
index 0b2c27a..f0010d0 100644
--- a/fs/ocfs2/journal.c
+++ b/fs/ocfs2/journal.c
@@ -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..1934999 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, "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: ");
+	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 Fri, Jun 19, 2009 at 04:53:19PM -0700, Sunil Mushran wrote:
> @@ -2392,11 +2398,13 @@ void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno, int ex)
>  	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, level);

	This should only be setting the LVB if the level was EX.

Joel

-- 

"Up and down that road in our worn out shoes,
 Talking bout good things and singing the blues."

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]

Good catch.

On Jun 19, 2009, at 9:41 PM, Joel Becker <Joel.Becker@oracle.com> wrote:

> On Fri, Jun 19, 2009 at 04:53:19PM -0700, Sunil Mushran wrote:
>> @@ -2392,11 +2398,13 @@ void ocfs2_orphan_scan_unlock(struct  
>> ocfs2_super *osb, u32 seqno, int ex)
>>    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, level);
>
>    This should only be setting the LVB if the level was EX.
>
> Joel
>
> -- 
>
> "Up and down that road in our worn out shoes,
> Talking bout good things and singing the blues."
>
> Joel Becker
> Principal Software Developer
> Oracle
> E-mail: joel.becker at oracle.com
> Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 3/5] ocfs2: Stop orphan scan as early as possible during umount

[Joel Becker]

On Fri, Jun 19, 2009 at 04:53:17PM -0700, Sunil Mushran wrote:
> Currently if the orphan scan fires a tick before the user issues the umount,
> the umount will wait for the queued orphan scan tasks to complete.
> 
> This patch makes the umount stop the orphan scan as early as possible so as
> to reduce the probability of the queued tasks slowing down the umount.
> 
> Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>

this is now in the fixes branch of ocfs2.git

Joel

-- 

"Reality is merely an illusion, albeit a very persistent one."
        - Albert Einstien

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

[Ocfs2-devel] [PATCH 4/5] ocfs2: Do not initialize lvb in ocfs2_orphan_scan_lock_res_init()

[Joel Becker]

On Fri, Jun 19, 2009 at 04:53:18PM -0700, Sunil Mushran wrote:
> No need to call ocfs2_dlm_lvb() in ocfs2_orphan_scan_lock_res_init().
> 
> Signed-off-by: Sunil Mushran <sunil.mushran@oracle.com>

This is now in the fixes branch of ocfs2.git.

Joel


-- 

Viro's Razor:
	Any race condition, no matter how unlikely, will occur just
	often enough to bite you.

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

[Joel Becker]

On Fri, Jun 19, 2009 at 09:41:30PM -0700, Joel Becker wrote:
> On Fri, Jun 19, 2009 at 04:53:19PM -0700, Sunil Mushran wrote:
> > @@ -2392,11 +2398,13 @@ void ocfs2_orphan_scan_unlock(struct ocfs2_super *osb, u32 seqno, int ex)
> >  	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, level);
> 
> 	This should only be setting the LVB if the level was EX.

	Actually, this lock is only ever taken as EX.  Why is there even
an ex argument to the function?

Joel

-- 

Life's Little Instruction Book #30

	"Never buy a house without a fireplace."

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:
> 	Actually, this lock is only ever taken as EX.  Why is there even
> an ex argument to the function?
>   
Yes, it is not needed. I can't see a need for PR access.
Fix is in testing.