[lustre-devel] [PATCH 23/29] lustre: osc_cache: remove 'transient' arg from osc_enter_cache_try

Andreas Dilger adilger at whamcloud.com
Wed Jan 9 19:02:45 PST 2019


On Jan 8, 2019, at 23:24, NeilBrown <neilb at suse.com> wrote:
> 
> This arg is always '0', so remove it.
> 
> Signed-off-by: NeilBrown <neilb at suse.com>

Out of curiosity, how would you find something like this?  Just
through code reading, or do you have some sort of static analysis
tool that shows this is dead code?

Digging into this a bit more, it looks like this is the only place
that increments cl_dirty_transit and sets OBD_BRW_NOCACHE, which
means the corresponding code in osc_release_write_grant() that checks
OBD_BRW_NOCACHE and decrements cl_dirty_transit is also dead, which
is good otherwise there would have been some kind of accounting leak.

That further implies that cl_dirty_transit is unused and can be removed,
along with obd_dirty_transit_pages.  The OBD_BRW_NOCACHE flag is part
of the wire protocol, but it looks like this was never actually sent on
the wire, just an internal housekeeping flag, so it should be marked like:

/* #define OBD_BRW_NOCACHE		0x400 internal use only 2.2.57-2.12 */

The follow-on cleanup could be part of this patch or a separate one.

Reviewed-by: Andreas Dilger <adilger at whamcloud.com>

> ---
> drivers/staging/lustre/lustre/osc/osc_cache.c |   11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
> index 8a68d3eb9314..b4bb36926046 100644
> --- a/drivers/staging/lustre/lustre/osc/osc_cache.c
> +++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
> @@ -1535,7 +1535,7 @@ static void osc_exit_cache(struct client_obd *cli, struct osc_async_page *oap)
>  */
> static bool osc_enter_cache_try(struct client_obd *cli,
> 				struct osc_async_page *oap,
> -				int bytes, int transient)
> +				int bytes)
> {
> 	OSC_DUMP_GRANT(D_CACHE, cli, "need:%d\n", bytes);
> 
> @@ -1545,11 +1545,6 @@ static bool osc_enter_cache_try(struct client_obd *cli,
> 	if (cli->cl_dirty_pages < cli->cl_dirty_max_pages &&
> 	    atomic_long_read(&obd_dirty_pages) + 1 <= obd_max_dirty_pages) {
> 		osc_consume_write_grant(cli, &oap->oap_brw_page);
> -		if (transient) {
> -			cli->cl_dirty_transit++;
> -			atomic_long_inc(&obd_dirty_transit_pages);
> -			oap->oap_brw_flags |= OBD_BRW_NOCACHE;
> -		}
> 		return true;
> 	} else {
> 		__osc_unreserve_grant(cli, bytes, bytes);
> @@ -1618,7 +1613,7 @@ static int osc_enter_cache(const struct lu_env *env, struct client_obd *cli,
> 	remain = wait_event_idle_exclusive_timeout_cmd(
> 		cli->cl_cache_waiters,
> 		(entered = osc_enter_cache_try(
> -			cli, oap, bytes, 0)) ||
> +			cli, oap, bytes)) ||
> 		(cli->cl_dirty_pages == 0 &&
> 		 cli->cl_w_in_flight == 0),
> 		timeout,
> @@ -2396,7 +2391,7 @@ int osc_queue_async_io(const struct lu_env *env, struct cl_io *io,
> 
> 		/* it doesn't need any grant to dirty this page */
> 		spin_lock(&cli->cl_loi_list_lock);
> -		if (!osc_enter_cache_try(cli, oap, grants, 0)) {
> +		if (!osc_enter_cache_try(cli, oap, grants)) {
> 			grants = 0;
> 			need_release = 1;
> 		}
> 
> 

Cheers, Andreas
---
Andreas Dilger
CTO Whamcloud






More information about the lustre-devel mailing list