linux-kernel - Re: [PATCH 10/14] dlm: don't use idr_remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130201001841.GP6824@mtj.dyndns.org>
Date:	Thu, 31 Jan 2013 16:18:41 -0800
From:	Tejun Heo <tj@...nel.org>
To:	David Teigland <teigland@...hat.com>
Cc:	akpm@...ux-foundation.org, rusty@...tcorp.com.au,
	linux-kernel@...r.kernel.org,
	Christine Caulfield <ccaulfie@...hat.com>,
	cluster-devel@...hat.com
Subject: Re: [PATCH 10/14] dlm: don't use idr_remove_all()

Hello, David.

On Thu, Jan 31, 2013 at 03:53:20PM -0800, Tejun Heo wrote:
> The function description is misleading.  The function does search
> inclusive range and needs explicit cursor increment to make progress.
> Weird that it doesn't work.  Looking into it.  I'll write when I know
> more.

It looks a bit weird to me that ls->ls_recover_list_count is also
incremented by recover_list_add().  The two code paths don't seem to
be interlocke at least upon my very shallow glance.  Is it that only
either the list or idr is in use?

Anyways, can you please apply the following patch and see which IDs
are leaking from the iteration?  The patch too is only compile tested
so I might have done something stupid but it hopefully shouldn't be
too difficult to make it work.

Thanks!
---
 fs/dlm/recover.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

--- a/fs/dlm/recover.c
+++ b/fs/dlm/recover.c
@@ -351,6 +351,12 @@ static struct dlm_rsb *recover_idr_find(
 	return r;
 }
 
+static int dlm_dump_idr(int id, void *p, void *data)
+{
+	pr_cont(" %d", id);
+	return 0;
+}
+
 static void recover_idr_clear(struct dlm_ls *ls)
 {
 	struct dlm_rsb *r;
@@ -358,7 +364,9 @@ static void recover_idr_clear(struct dlm
 
 	spin_lock(&ls->ls_recover_idr_lock);
 
+	pr_info("XXX clearing:");
 	idr_for_each_entry(&ls->ls_recover_idr, r, id) {
+		pr_cont(" %d", id);
 		idr_remove(&ls->ls_recover_idr, id);
 		r->res_id = 0;
 		r->res_recover_locks_count = 0;
@@ -366,10 +374,14 @@ static void recover_idr_clear(struct dlm
 
 		dlm_put_rsb(r);
 	}
+	pr_cont("\n");
 
 	if (ls->ls_recover_list_count != 0) {
 		log_error(ls, "warning: recover_list_count %d",
 			  ls->ls_recover_list_count);
+		pr_info("XXX leftovers: ");
+		idr_for_each(&ls->ls_recover_idr, dlm_dump_idr, NULL);
+		pr_cont("\n");
 		ls->ls_recover_list_count = 0;
 	}
 	spin_unlock(&ls->ls_recover_idr_lock);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/