Re: [PATCH 10/14] dlm: don't use idr_remove_all()
Hello, David. On Fri, Feb 01, 2013 at 12:44:43PM -0500, David Teigland wrote: > I already tried my own version of this, but used idr_for_each_entry a > second time. Unfortunately, the number it found and printed did not match > recover_list_count. > > warning: recover_list_count 566 > > It printed 304 ids: Heh, that's really weird. Maybe idr_get_next() is buggy above certain number or at certain boundaries? If you find something, please let me know. I'll see if I can reproduce it in some minimal way. Thanks! -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Thu, Jan 31, 2013 at 04:18:41PM -0800, Tejun Heo wrote: > 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? Yes, that's correct. > 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. I'm trying your patch now, I don't have a test optimized to hit this code so it may take a while. > 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_recover_idr_lock); > > + pr_info("XXX clearing:"); > idr_for_each_entry(>ls_recover_idr, r, id) { > + pr_cont(" %d", id); It will often be clearing hundreds of thousands of entries, so this will probably be excessive. > 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_recover_idr, dlm_dump_idr, NULL); > + pr_cont("\n"); I already tried my own version of this, but used idr_for_each_entry a second time. Unfortunately, the number it found and printed did not match recover_list_count. warning: recover_list_count 566 It printed 304 ids: 41218 41222 41223 41224 41226 41228 41229 41230 41231 41232 41234 41235 41236 41237 41239 41241 41242 41243 41244 41245 41246 41249 41252 41253 41254 41255 41256 41257 41259 41260 41261 41263 41264 41266 41271 41272 41273 41274 41277 41278 41475 41480 41483 41524 41525 41526 41655 41731 41741 41745 41749 41767 41768 41769 41772 41773 41782 42113 42114 42115 42121 42122 42124 42128 42132 42136 42138 42139 42141 42165 42375 42381 42385 42388 42390 42392 42399 42401 42404 42407 42409 42411 42416 42422 42694 42699 42712 42717 42727 42866 43009 43042 43044 43046 43049 43051 43058 43059 43064 43065 43066 43067 43330 43332 43337 43338 43339 43343 43348 43349 43351 43354 43355 43356 43361 43362 43368 43369 43370 43375 43376 43377 43378 43379 43381 43575 43576 43577 43677 43678 43680 43683 43684 43685 43689 43690 43819 43820 43823 43824 43825 43826 43827 43828 43829 43831 43905 43907 43908 43912 43929 43930 43955 43956 43960 43962 43965 44288 44289 44291 44296 44298 44300 44310 44311 44313 44314 44316 44318 44321 44323 44325 44454 44456 44457 44458 44544 44547 44548 44550 44555 44557 44560 44562 44564 44567 44573 44575 44576 44578 44579 44581 44582 44583 44584 44585 44589 44592 44595 44596 44726 44728 44729 44732 44734 44866 44867 44873 44876 44878 44879 44912 44914 44916 44920 44923 44924 45053 45186 45189 45190 45195 45197 45199 45200 45201 45203 45204 45208 45209 45212 45213 45216 45220 45223 45224 45225 45227 45228 45231 45234 45440 45441 45444 45448 45450 45452 45454 45456 45457 45458 45459 45460 45461 45464 45466 45467 45472 45475 45477 45484 45485 45488 45492 45494 45495 45496 45497 45498 45499 45628 45630 45698 45699 45700 45703 45707 45708 45710 45713 45715 45717 45720 45722 45723 45724 45725 45727 45729 45730 45731 45733 45734 45737 45739 45741 45742 45746 45748 45750 45755 47292 47293 47294 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Thu, Jan 31, 2013 at 04:18:41PM -0800, Tejun Heo wrote: 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? Yes, that's correct. 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. I'm trying your patch now, I don't have a test optimized to hit this code so it may take a while. 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); It will often be clearing hundreds of thousands of entries, so this will probably be excessive. 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); I already tried my own version of this, but used idr_for_each_entry a second time. Unfortunately, the number it found and printed did not match recover_list_count. warning: recover_list_count 566 It printed 304 ids: 41218 41222 41223 41224 41226 41228 41229 41230 41231 41232 41234 41235 41236 41237 41239 41241 41242 41243 41244 41245 41246 41249 41252 41253 41254 41255 41256 41257 41259 41260 41261 41263 41264 41266 41271 41272 41273 41274 41277 41278 41475 41480 41483 41524 41525 41526 41655 41731 41741 41745 41749 41767 41768 41769 41772 41773 41782 42113 42114 42115 42121 42122 42124 42128 42132 42136 42138 42139 42141 42165 42375 42381 42385 42388 42390 42392 42399 42401 42404 42407 42409 42411 42416 42422 42694 42699 42712 42717 42727 42866 43009 43042 43044 43046 43049 43051 43058 43059 43064 43065 43066 43067 43330 43332 43337 43338 43339 43343 43348 43349 43351 43354 43355 43356 43361 43362 43368 43369 43370 43375 43376 43377 43378 43379 43381 43575 43576 43577 43677 43678 43680 43683 43684 43685 43689 43690 43819 43820 43823 43824 43825 43826 43827 43828 43829 43831 43905 43907 43908 43912 43929 43930 43955 43956 43960 43962 43965 44288 44289 44291 44296 44298 44300 44310 44311 44313 44314 44316 44318 44321 44323 44325 44454 44456 44457 44458 44544 44547 44548 44550 44555 44557 44560 44562 44564 44567 44573 44575 44576 44578 44579 44581 44582 44583 44584 44585 44589 44592 44595 44596 44726 44728 44729 44732 44734 44866 44867 44873 44876 44878 44879 44912 44914 44916 44920 44923 44924 45053 45186 45189 45190 45195 45197 45199 45200 45201 45203 45204 45208 45209 45212 45213 45216 45220 45223 45224 45225 45227 45228 45231 45234 45440 45441 45444 45448 45450 45452 45454 45456 45457 45458 45459 45460 45461 45464 45466 45467 45472 45475 45477 45484 45485 45488 45492 45494 45495 45496 45497 45498 45499 45628 45630 45698 45699 45700 45703 45707 45708 45710 45713 45715 45717 45720 45722 45723 45724 45725 45727 45729 45730 45731 45733 45734 45737 45739 45741 45742 45746 45748 45750 45755 47292 47293 47294 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
Hello, David. On Fri, Feb 01, 2013 at 12:44:43PM -0500, David Teigland wrote: I already tried my own version of this, but used idr_for_each_entry a second time. Unfortunately, the number it found and printed did not match recover_list_count. warning: recover_list_count 566 It printed 304 ids: Heh, that's really weird. Maybe idr_get_next() is buggy above certain number or at certain boundaries? If you find something, please let me know. I'll see if I can reproduce it in some minimal way. Thanks! -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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_recover_idr_lock); + pr_info("XXX clearing:"); idr_for_each_entry(>ls_recover_idr, r, id) { + pr_cont(" %d", id); idr_remove(>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_recover_idr, dlm_dump_idr, NULL); + pr_cont("\n"); ls->ls_recover_list_count = 0; } spin_unlock(>ls_recover_idr_lock); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
Hello, David. Sorry about the delay. On Wed, Jan 30, 2013 at 04:24:18PM -0500, David Teigland wrote: > > Unfortunately, the list_for_each_entry doesn't seem to be clearing > > everything. I've seen "warning: recover_list_count 39" at the end of that > > function. > > I don't want to pretend to understand the internals of this idr code, but > it's not clear that idr_for_each is equivalent to idr_for_each_entry when > iterating through all id values. The "++id" in idr_for_each_entry looks > like it could lead to some missed entries? The comment about idr_get_next > returning the "next number to given id" sounds like an entry with an id of > "++id" would be missed. 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. Thanks! -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
Hello, David. Sorry about the delay. On Wed, Jan 30, 2013 at 04:24:18PM -0500, David Teigland wrote: Unfortunately, the list_for_each_entry doesn't seem to be clearing everything. I've seen warning: recover_list_count 39 at the end of that function. I don't want to pretend to understand the internals of this idr code, but it's not clear that idr_for_each is equivalent to idr_for_each_entry when iterating through all id values. The ++id in idr_for_each_entry looks like it could lead to some missed entries? The comment about idr_get_next returning the next number to given id sounds like an entry with an id of ++id would be missed. 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. Thanks! -- tejun -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Tue, Jan 29, 2013 at 10:13:17AM -0500, David Teigland wrote: > On Mon, Jan 28, 2013 at 10:57:23AM -0500, David Teigland wrote: > > On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: > > > idr_destroy() can destroy idr by itself and idr_remove_all() is being > > > deprecated. > > > > > > The conversion isn't completely trivial for recover_idr_clear() as > > > it's the only place in kernel which makes legitimate use of > > > idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call > > > inside idr_for_each_entry() loop. It goes on top so that it matches > > > the operation order in recover_idr_del(). > > > > > > Only compile tested. > > > > > > Signed-off-by: Tejun Heo > > > Cc: Christine Caulfield > > > Cc: David Teigland > > > Cc: cluster-de...@redhat.com > > > --- > > > This patch depends on an earlier idr patch and given the trivial > > > nature of the patch, I think it would be best to route these together > > > through -mm. Please holler if there's any objection. > > > > Yes, that's good for me. I'll grab the set and test the dlm bits. > > Hi Tejun, > Unfortunately, the list_for_each_entry doesn't seem to be clearing > everything. I've seen "warning: recover_list_count 39" at the end of that > function. I don't want to pretend to understand the internals of this idr code, but it's not clear that idr_for_each is equivalent to idr_for_each_entry when iterating through all id values. The "++id" in idr_for_each_entry looks like it could lead to some missed entries? The comment about idr_get_next returning the "next number to given id" sounds like an entry with an id of "++id" would be missed. Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Tue, Jan 29, 2013 at 10:13:17AM -0500, David Teigland wrote: On Mon, Jan 28, 2013 at 10:57:23AM -0500, David Teigland wrote: On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. The conversion isn't completely trivial for recover_idr_clear() as it's the only place in kernel which makes legitimate use of idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call inside idr_for_each_entry() loop. It goes on top so that it matches the operation order in recover_idr_del(). Only compile tested. Signed-off-by: Tejun Heo t...@kernel.org Cc: Christine Caulfield ccaul...@redhat.com Cc: David Teigland teigl...@redhat.com Cc: cluster-de...@redhat.com --- This patch depends on an earlier idr patch and given the trivial nature of the patch, I think it would be best to route these together through -mm. Please holler if there's any objection. Yes, that's good for me. I'll grab the set and test the dlm bits. Hi Tejun, Unfortunately, the list_for_each_entry doesn't seem to be clearing everything. I've seen warning: recover_list_count 39 at the end of that function. I don't want to pretend to understand the internals of this idr code, but it's not clear that idr_for_each is equivalent to idr_for_each_entry when iterating through all id values. The ++id in idr_for_each_entry looks like it could lead to some missed entries? The comment about idr_get_next returning the next number to given id sounds like an entry with an id of ++id would be missed. Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Mon, Jan 28, 2013 at 10:57:23AM -0500, David Teigland wrote: > On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: > > idr_destroy() can destroy idr by itself and idr_remove_all() is being > > deprecated. > > > > The conversion isn't completely trivial for recover_idr_clear() as > > it's the only place in kernel which makes legitimate use of > > idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call > > inside idr_for_each_entry() loop. It goes on top so that it matches > > the operation order in recover_idr_del(). > > > > Only compile tested. > > > > Signed-off-by: Tejun Heo > > Cc: Christine Caulfield > > Cc: David Teigland > > Cc: cluster-de...@redhat.com > > --- > > This patch depends on an earlier idr patch and given the trivial > > nature of the patch, I think it would be best to route these together > > through -mm. Please holler if there's any objection. > > Yes, that's good for me. I'll grab the set and test the dlm bits. Hi Tejun, Unfortunately, the list_for_each_entry doesn't seem to be clearing everything. I've seen "warning: recover_list_count 39" at the end of that function. Dave -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Mon, Jan 28, 2013 at 10:57:23AM -0500, David Teigland wrote: On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. The conversion isn't completely trivial for recover_idr_clear() as it's the only place in kernel which makes legitimate use of idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call inside idr_for_each_entry() loop. It goes on top so that it matches the operation order in recover_idr_del(). Only compile tested. Signed-off-by: Tejun Heo t...@kernel.org Cc: Christine Caulfield ccaul...@redhat.com Cc: David Teigland teigl...@redhat.com Cc: cluster-de...@redhat.com --- This patch depends on an earlier idr patch and given the trivial nature of the patch, I think it would be best to route these together through -mm. Please holler if there's any objection. Yes, that's good for me. I'll grab the set and test the dlm bits. Hi Tejun, Unfortunately, the list_for_each_entry doesn't seem to be clearing everything. I've seen warning: recover_list_count 39 at the end of that function. Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: > idr_destroy() can destroy idr by itself and idr_remove_all() is being > deprecated. > > The conversion isn't completely trivial for recover_idr_clear() as > it's the only place in kernel which makes legitimate use of > idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call > inside idr_for_each_entry() loop. It goes on top so that it matches > the operation order in recover_idr_del(). > > Only compile tested. > > Signed-off-by: Tejun Heo > Cc: Christine Caulfield > Cc: David Teigland > Cc: cluster-de...@redhat.com > --- > This patch depends on an earlier idr patch and given the trivial > nature of the patch, I think it would be best to route these together > through -mm. Please holler if there's any objection. Yes, that's good for me. I'll grab the set and test the dlm bits. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 10/14] dlm: don't use idr_remove_all()
On Fri, Jan 25, 2013 at 05:31:08PM -0800, Tejun Heo wrote: idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. The conversion isn't completely trivial for recover_idr_clear() as it's the only place in kernel which makes legitimate use of idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call inside idr_for_each_entry() loop. It goes on top so that it matches the operation order in recover_idr_del(). Only compile tested. Signed-off-by: Tejun Heo t...@kernel.org Cc: Christine Caulfield ccaul...@redhat.com Cc: David Teigland teigl...@redhat.com Cc: cluster-de...@redhat.com --- This patch depends on an earlier idr patch and given the trivial nature of the patch, I think it would be best to route these together through -mm. Please holler if there's any objection. Yes, that's good for me. I'll grab the set and test the dlm bits. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/14] dlm: don't use idr_remove_all()
idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. The conversion isn't completely trivial for recover_idr_clear() as it's the only place in kernel which makes legitimate use of idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call inside idr_for_each_entry() loop. It goes on top so that it matches the operation order in recover_idr_del(). Only compile tested. Signed-off-by: Tejun Heo Cc: Christine Caulfield Cc: David Teigland Cc: cluster-de...@redhat.com --- This patch depends on an earlier idr patch and given the trivial nature of the patch, I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. fs/dlm/lockspace.c | 1 - fs/dlm/recover.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index 2e99fb0..3ca79d3 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -796,7 +796,6 @@ static int release_lockspace(struct dlm_ls *ls, int force) */ idr_for_each(>ls_lkbidr, lkb_idr_free, ls); - idr_remove_all(>ls_lkbidr); idr_destroy(>ls_lkbidr); /* diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c index b2856e7..236d108 100644 --- a/fs/dlm/recover.c +++ b/fs/dlm/recover.c @@ -359,13 +359,13 @@ static void recover_idr_clear(struct dlm_ls *ls) spin_lock(>ls_recover_idr_lock); idr_for_each_entry(>ls_recover_idr, r, id) { + idr_remove(>ls_recover_idr, id); r->res_id = 0; r->res_recover_locks_count = 0; ls->ls_recover_list_count--; dlm_put_rsb(r); } - idr_remove_all(>ls_recover_idr); if (ls->ls_recover_list_count != 0) { log_error(ls, "warning: recover_list_count %d", -- 1.8.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 10/14] dlm: don't use idr_remove_all()
idr_destroy() can destroy idr by itself and idr_remove_all() is being deprecated. The conversion isn't completely trivial for recover_idr_clear() as it's the only place in kernel which makes legitimate use of idr_remove_all() w/o idr_destroy(). Replace it with idr_remove() call inside idr_for_each_entry() loop. It goes on top so that it matches the operation order in recover_idr_del(). Only compile tested. Signed-off-by: Tejun Heo t...@kernel.org Cc: Christine Caulfield ccaul...@redhat.com Cc: David Teigland teigl...@redhat.com Cc: cluster-de...@redhat.com --- This patch depends on an earlier idr patch and given the trivial nature of the patch, I think it would be best to route these together through -mm. Please holler if there's any objection. Thanks. fs/dlm/lockspace.c | 1 - fs/dlm/recover.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c index 2e99fb0..3ca79d3 100644 --- a/fs/dlm/lockspace.c +++ b/fs/dlm/lockspace.c @@ -796,7 +796,6 @@ static int release_lockspace(struct dlm_ls *ls, int force) */ idr_for_each(ls-ls_lkbidr, lkb_idr_free, ls); - idr_remove_all(ls-ls_lkbidr); idr_destroy(ls-ls_lkbidr); /* diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c index b2856e7..236d108 100644 --- a/fs/dlm/recover.c +++ b/fs/dlm/recover.c @@ -359,13 +359,13 @@ static void recover_idr_clear(struct dlm_ls *ls) spin_lock(ls-ls_recover_idr_lock); idr_for_each_entry(ls-ls_recover_idr, r, id) { + idr_remove(ls-ls_recover_idr, id); r-res_id = 0; r-res_recover_locks_count = 0; ls-ls_recover_list_count--; dlm_put_rsb(r); } - idr_remove_all(ls-ls_recover_idr); if (ls-ls_recover_list_count != 0) { log_error(ls, warning: recover_list_count %d, -- 1.8.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/