Re: [PATCH 1/2] Revert "IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs"
Diabolical ;-) Thanks for the pointer! Benny On Feb. 26, 2008, 11:39 -0800, Matthew Wilcox <[EMAIL PROTECTED]> wrote: > On Tue, Feb 26, 2008 at 11:23:01AM -0800, Benny Halevy wrote: >> Pete, the subject says "PATCH 1/2" but I didn't see any follow-up message >> for PATCH 2/2. Just wondering :) > > I think the problem's on your end ... I got it and so did marc: > http://marc.info/?l=linux-scsi&m=120405067313933&w=2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] Revert "IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs"
On Tue, Feb 26, 2008 at 11:23:01AM -0800, Benny Halevy wrote: > Pete, the subject says "PATCH 1/2" but I didn't see any follow-up message > for PATCH 2/2. Just wondering :) I think the problem's on your end ... I got it and so did marc: http://marc.info/?l=linux-scsi&m=120405067313933&w=2 -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] Revert "IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs"
Pete, the subject says "PATCH 1/2" but I didn't see any follow-up message
for PATCH 2/2. Just wondering :)
Benny
On Feb. 26, 2008, 10:27 -0800, Pete Wyckoff <[EMAIL PROTECTED]> wrote:
> This reverts commit a3cd7d9070be417a21905c997ee32d756d999b38.
>
> The original commit breaks iSER reliably, making it complain:
>
> iser: iser_reg_page_vec:ib_fmr_pool_map_phys failed: -11
>
> The FMR cleanup thread runs ib_fmr_batch_release() as dirty
> entries build up. This commit causes clean but used FMR
> entries also to be purged. During that process, another thread
> can see that there are no free FMRs and fail, even though
> there should always have been enough available.
>
> Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
> ---
> drivers/infiniband/core/fmr_pool.c | 21 ++---
> 1 files changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/infiniband/core/fmr_pool.c
> b/drivers/infiniband/core/fmr_pool.c
> index 7f00347..4044fdf 100644
> --- a/drivers/infiniband/core/fmr_pool.c
> +++ b/drivers/infiniband/core/fmr_pool.c
> @@ -139,7 +139,7 @@ static inline struct ib_pool_fmr
> *ib_fmr_cache_lookup(struct ib_fmr_pool *pool,
> static void ib_fmr_batch_release(struct ib_fmr_pool *pool)
> {
> int ret;
> - struct ib_pool_fmr *fmr, *next;
> + struct ib_pool_fmr *fmr;
> LIST_HEAD(unmap_list);
> LIST_HEAD(fmr_list);
>
> @@ -158,20 +158,6 @@ static void ib_fmr_batch_release(struct ib_fmr_pool
> *pool)
> #endif
> }
>
> - /*
> - * The free_list may hold FMRs that have been put there
> - * because they haven't reached the max_remap count.
> - * Invalidate their mapping as well.
> - */
> - list_for_each_entry_safe(fmr, next, &pool->free_list, list) {
> - if (fmr->remap_count == 0)
> - continue;
> - hlist_del_init(&fmr->cache_node);
> - fmr->remap_count = 0;
> - list_add_tail(&fmr->fmr->list, &fmr_list);
> - list_move(&fmr->list, &unmap_list);
> - }
> -
> list_splice(&pool->dirty_list, &unmap_list);
> INIT_LIST_HEAD(&pool->dirty_list);
> pool->dirty_len = 0;
> @@ -384,6 +370,11 @@ void ib_destroy_fmr_pool(struct ib_fmr_pool *pool)
>
> i = 0;
> list_for_each_entry_safe(fmr, tmp, &pool->free_list, list) {
> + if (fmr->remap_count) {
> + INIT_LIST_HEAD(&fmr_list);
> + list_add_tail(&fmr->fmr->list, &fmr_list);
> + ib_unmap_fmr(&fmr_list);
> + }
> ib_dealloc_fmr(fmr->fmr);
> list_del(&fmr->list);
> kfree(fmr);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] Revert "IB/fmr_pool: ib_fmr_pool_flush() should flush all dirty FMRs"
This reverts commit a3cd7d9070be417a21905c997ee32d756d999b38.
The original commit breaks iSER reliably, making it complain:
iser: iser_reg_page_vec:ib_fmr_pool_map_phys failed: -11
The FMR cleanup thread runs ib_fmr_batch_release() as dirty
entries build up. This commit causes clean but used FMR
entries also to be purged. During that process, another thread
can see that there are no free FMRs and fail, even though
there should always have been enough available.
Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
---
drivers/infiniband/core/fmr_pool.c | 21 ++---
1 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/infiniband/core/fmr_pool.c
b/drivers/infiniband/core/fmr_pool.c
index 7f00347..4044fdf 100644
--- a/drivers/infiniband/core/fmr_pool.c
+++ b/drivers/infiniband/core/fmr_pool.c
@@ -139,7 +139,7 @@ static inline struct ib_pool_fmr
*ib_fmr_cache_lookup(struct ib_fmr_pool *pool,
static void ib_fmr_batch_release(struct ib_fmr_pool *pool)
{
int ret;
- struct ib_pool_fmr *fmr, *next;
+ struct ib_pool_fmr *fmr;
LIST_HEAD(unmap_list);
LIST_HEAD(fmr_list);
@@ -158,20 +158,6 @@ static void ib_fmr_batch_release(struct ib_fmr_pool *pool)
#endif
}
- /*
-* The free_list may hold FMRs that have been put there
-* because they haven't reached the max_remap count.
-* Invalidate their mapping as well.
-*/
- list_for_each_entry_safe(fmr, next, &pool->free_list, list) {
- if (fmr->remap_count == 0)
- continue;
- hlist_del_init(&fmr->cache_node);
- fmr->remap_count = 0;
- list_add_tail(&fmr->fmr->list, &fmr_list);
- list_move(&fmr->list, &unmap_list);
- }
-
list_splice(&pool->dirty_list, &unmap_list);
INIT_LIST_HEAD(&pool->dirty_list);
pool->dirty_len = 0;
@@ -384,6 +370,11 @@ void ib_destroy_fmr_pool(struct ib_fmr_pool *pool)
i = 0;
list_for_each_entry_safe(fmr, tmp, &pool->free_list, list) {
+ if (fmr->remap_count) {
+ INIT_LIST_HEAD(&fmr_list);
+ list_add_tail(&fmr->fmr->list, &fmr_list);
+ ib_unmap_fmr(&fmr_list);
+ }
ib_dealloc_fmr(fmr->fmr);
list_del(&fmr->list);
kfree(fmr);
--
1.5.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
