netdev - Re: [PATCH net-next 1/7] net/rds: Give fr_state a chance to transition to FRMR_IS

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <20190701.112751.509316780582361121.davem@davemloft.net>
Date:   Mon, 01 Jul 2019 11:27:51 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     gerd.rausch@...cle.com
Cc:     santosh.shilimkar@...cle.com, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/7] net/rds: Give fr_state a chance to
 transition to FRMR_IS_FREE

From: Gerd Rausch <gerd.rausch@...cle.com>
Date: Mon, 1 Jul 2019 09:39:44 -0700

> +			/* Memory regions make it onto the "clean_list" via
> +			 * "rds_ib_flush_mr_pool", after the memory region has
> +			 * been posted for invalidation via "rds_ib_post_inv".
> +			 *
> +			 * At that point in time, "fr_state" may still be
> +			 * in state "FRMR_IS_INUSE", since the only place where
> +			 * "fr_state" transitions to "FRMR_IS_FREE" is in
> +			 * is in "rds_ib_mr_cqe_handler", which is
> +			 * triggered by a tasklet.
> +			 *
> +			 * So in case we notice that
> +			 * "fr_state != FRMR_IS_FREE" (see below), * we wait for
> +			 * "fr_inv_done" to trigger with a maximum of 10msec.
> +			 * Then we check again, and only put the memory region
> +			 * onto the drop_list (via "rds_ib_free_frmr")
> +			 * in case the situation remains unchanged.
> +			 *
> +			 * This avoids the problem of memory-regions bouncing
> +			 * between "clean_list" and "drop_list" before they
> +			 * even have a chance to be properly invalidated.
> +			 */
> +			frmr = &ibmr->u.frmr;
> +			wait_event_timeout(frmr->fr_inv_done,
> +					   frmr->fr_state == FRMR_IS_FREE,
> +					   msecs_to_jiffies(10));
> +			if (frmr->fr_state == FRMR_IS_FREE)
> +				break;

If we see FRMR_IS_FREE after the timeout, what cleans this up?

Also, why 10msec?  Why that specific value and not some other value?  Why
not wait for however long it takes for the tasklet to run and clean it up?