linux-kernel - Re: [PATCH 10/19] staging: lustre: simplify waiting in ptlrpc_invalidate

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: <alpine.LFD.2.21.1801171532260.11282@casper.infradead.org>
Date:   Wed, 17 Jan 2018 15:32:38 +0000 (GMT)
From:   James Simmons <jsimmons@...radead.org>
To:     NeilBrown <neilb@...e.com>
cc:     Oleg Drokin <oleg.drokin@...el.com>,
        Andreas Dilger <andreas.dilger@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        lkml <linux-kernel@...r.kernel.org>,
        lustre <lustre-devel@...ts.lustre.org>
Subject: Re: [PATCH 10/19] staging: lustre: simplify waiting in
 ptlrpc_invalidate_import()


> This waiter currently wakes up every second to re-test if
> imp_flight is zero.  If we ensure wakeup is called whenever
> imp_flight is decremented to zero, we can just have a simple
> wait_event_idle_timeout().
> 
> So add a wake_up_all to the one place it is missing, and simplify
> the wait_event.

Reviewed-by: James Simmons <jsimmons@...radead.org>
 
> Signed-off-by: NeilBrown <neilb@...e.com>
> ---
>  drivers/staging/lustre/lustre/ptlrpc/client.c |    3 ++-
>  drivers/staging/lustre/lustre/ptlrpc/import.c |   21 ++++++++-------------
>  2 files changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
> index 0ab13f8e5993..81b7a7046d82 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/client.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
> @@ -1588,7 +1588,8 @@ static int ptlrpc_send_new_req(struct ptlrpc_request *req)
>  		spin_lock(&imp->imp_lock);
>  		if (!list_empty(&req->rq_list)) {
>  			list_del_init(&req->rq_list);
> -			atomic_dec(&req->rq_import->imp_inflight);
> +			if (atomic_dec_and_test(&req->rq_import->imp_inflight))
> +				wake_up_all(&req->rq_import->imp_recovery_waitq);
>  		}
>  		spin_unlock(&imp->imp_lock);
>  		ptlrpc_rqphase_move(req, RQ_PHASE_NEW);
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c b/drivers/staging/lustre/lustre/ptlrpc/import.c
> index ed210550f61f..5d62c9de27eb 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/import.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/import.c
> @@ -265,7 +265,6 @@ void ptlrpc_invalidate_import(struct obd_import *imp)
>  {
>  	struct list_head *tmp, *n;
>  	struct ptlrpc_request *req;
> -	struct l_wait_info lwi;
>  	unsigned int timeout;
>  	int rc;
>  
> @@ -306,19 +305,15 @@ void ptlrpc_invalidate_import(struct obd_import *imp)
>  		 * callbacks. Cap it at obd_timeout -- these should all
>  		 * have been locally cancelled by ptlrpc_abort_inflight.
>  		 */
> -		lwi = LWI_TIMEOUT_INTERVAL(
> -			cfs_timeout_cap(timeout * HZ),
> -			(timeout > 1) ? HZ :
> -			HZ / 2,
> -			NULL, NULL);
> -		rc = l_wait_event(imp->imp_recovery_waitq,
> -				  (atomic_read(&imp->imp_inflight) == 0),
> -				  &lwi);
> -		if (rc) {
> +		rc = wait_event_idle_timeout(imp->imp_recovery_waitq,
> +					     atomic_read(&imp->imp_inflight) == 0,
> +					     obd_timeout * HZ);
> +
> +		if (rc == 0) {
>  			const char *cli_tgt = obd2cli_tgt(imp->imp_obd);
>  
> -			CERROR("%s: rc = %d waiting for callback (%d != 0)\n",
> -			       cli_tgt, rc,
> +			CERROR("%s: timeout waiting for callback (%d != 0)\n",
> +			       cli_tgt,
>  			       atomic_read(&imp->imp_inflight));
>  
>  			spin_lock(&imp->imp_lock);
> @@ -365,7 +360,7 @@ void ptlrpc_invalidate_import(struct obd_import *imp)
>  			}
>  			spin_unlock(&imp->imp_lock);
>  		}
> -	} while (rc != 0);
> +	} while (rc == 0);
>  
>  	/*
>  	 * Let's additionally check that no new rpcs added to import in
> 
> 
>