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

From: NeilBrown <neilb@suse.com>
To: lustre-devel@lists.lustre.org
Subject: [lustre-devel] [PATCH 23/29] lustre: osc_cache: remove 'transient' arg from osc_enter_cache_try
Date: Fri, 11 Jan 2019 11:27:38 +1100	[thread overview]
Message-ID: <87ftu0ghs5.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <87E1C21A-052C-46A7-81AA-2183EB3BB178@whamcloud.com>

On Thu, Jan 10 2019, Andreas Dilger wrote:

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

Below is the revised version of this patch.
Thanks,
NeilBrown

From: NeilBrown <neilb@suse.com>
Subject: [PATCH] lustre: osc_cache: remove 'transient' arg from
 osc_enter_cache_try

This arg is always '0', so remove it.
Consequently, OBD_BRW_NOCACHE is never set, and
 cl_dirty_transit and obd_dirty_transit_pages
are never non-zero.
So they can be removed as well.

Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Signed-off-by: NeilBrown <neilb@suse.com>
---
 .../lustre/include/uapi/linux/lustre/lustre_idl.h        |  2 +-
 drivers/staging/lustre/lustre/include/obd.h              |  1 -
 drivers/staging/lustre/lustre/include/obd_support.h      |  1 -
 drivers/staging/lustre/lustre/obdclass/class_obd.c       |  3 ---
 drivers/staging/lustre/lustre/osc/osc_cache.c            | 16 +++-------------
 drivers/staging/lustre/lustre/osc/osc_request.c          | 14 ++++++--------
 drivers/staging/lustre/lustre/ptlrpc/wiretest.c          |  2 --
 7 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
index a8de36c8fbe4..bffe62e87e00 100644
--- a/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
+++ b/drivers/staging/lustre/include/uapi/linux/lustre/lustre_idl.h
@@ -1182,7 +1182,7 @@ struct hsm_state_set {
 #define OBD_BRW_CHECK		0x10
 #define OBD_BRW_FROM_GRANT	0x20 /* the osc manages this under llite */
 #define OBD_BRW_GRANTED		0x40 /* the ost manages this */
-#define OBD_BRW_NOCACHE		0x80 /* this page is a part of non-cached IO */
+/* #define OBD_BRW_NOCACHE	0x80 internal use only 2.2.57-2.12 */
 #define OBD_BRW_NOQUOTA	       0x100
 #define OBD_BRW_SRVLOCK	       0x200 /* Client holds no lock over this page */
 #define OBD_BRW_ASYNC	       0x400 /* Server may delay commit to disk */
diff --git a/drivers/staging/lustre/lustre/include/obd.h b/drivers/staging/lustre/lustre/include/obd.h
index 0d8b9fe4bcec..4b43707f3d36 100644
--- a/drivers/staging/lustre/lustre/include/obd.h
+++ b/drivers/staging/lustre/lustre/include/obd.h
@@ -193,7 +193,6 @@ struct client_obd {
 	/* the grant values are protected by loi_list_lock below */
 	unsigned long		 cl_dirty_pages;	/* all _dirty_ in pages */
 	unsigned long		 cl_dirty_max_pages;	/* allowed w/o rpc */
-	unsigned long		 cl_dirty_transit;	/* dirty synchronous */
 	unsigned long		 cl_avail_grant;	/* bytes of credit for ost */
 	unsigned long		 cl_lost_grant;		/* lost credits (trunc) */
 	/* grant consumed for dirty pages */
diff --git a/drivers/staging/lustre/lustre/include/obd_support.h b/drivers/staging/lustre/lustre/include/obd_support.h
index 79875fad3f18..93a374514a77 100644
--- a/drivers/staging/lustre/lustre/include/obd_support.h
+++ b/drivers/staging/lustre/lustre/include/obd_support.h
@@ -55,7 +55,6 @@ extern int at_early_margin;
 extern int at_extra;
 extern unsigned long obd_max_dirty_pages;
 extern atomic_long_t obd_dirty_pages;
-extern atomic_long_t obd_dirty_transit_pages;
 extern char obd_jobid_var[];
 
 /* Some hash init argument constants */
diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c b/drivers/staging/lustre/lustre/obdclass/class_obd.c
index 5b0b2f64a4a3..e563ebb5b328 100644
--- a/drivers/staging/lustre/lustre/obdclass/class_obd.c
+++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c
@@ -77,9 +77,6 @@ EXPORT_SYMBOL(at_early_margin);
 int at_extra = 30;
 EXPORT_SYMBOL(at_extra);
 
-atomic_long_t obd_dirty_transit_pages;
-EXPORT_SYMBOL(obd_dirty_transit_pages);
-
 char obd_jobid_var[JOBSTATS_JOBID_VAR_MAX_LEN + 1] = JOBSTATS_DISABLE;
 char obd_jobid_node[LUSTRE_JOBID_SIZE + 1];
 
diff --git a/drivers/staging/lustre/lustre/osc/osc_cache.c b/drivers/staging/lustre/lustre/osc/osc_cache.c
index 8f5c567b4e15..53f067d4e09b 100644
--- a/drivers/staging/lustre/lustre/osc/osc_cache.c
+++ b/drivers/staging/lustre/lustre/osc/osc_cache.c
@@ -1424,11 +1424,6 @@ static void osc_release_write_grant(struct client_obd *cli,
 	pga->flag &= ~OBD_BRW_FROM_GRANT;
 	atomic_long_dec(&obd_dirty_pages);
 	cli->cl_dirty_pages--;
-	if (pga->flag & OBD_BRW_NOCACHE) {
-		pga->flag &= ~OBD_BRW_NOCACHE;
-		atomic_long_dec(&obd_dirty_transit_pages);
-		cli->cl_dirty_transit--;
-	}
 }
 
 /**
@@ -1535,7 +1530,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 +1540,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 +1608,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 +2386,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;
 		}
diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c
index b28fbacbcfbf..e18d592bf42a 100644
--- a/drivers/staging/lustre/lustre/osc/osc_request.c
+++ b/drivers/staging/lustre/lustre/osc/osc_request.c
@@ -678,22 +678,20 @@ static void osc_announce_cached(struct client_obd *cli, struct obdo *oa,
 		oa->o_dirty = cli->cl_dirty_grant;
 	else
 		oa->o_dirty = cli->cl_dirty_pages << PAGE_SHIFT;
-	if (unlikely(cli->cl_dirty_pages - cli->cl_dirty_transit >
+	if (unlikely(cli->cl_dirty_pages >
 		     cli->cl_dirty_max_pages)) {
-		CERROR("dirty %lu - %lu > dirty_max %lu\n",
-		       cli->cl_dirty_pages, cli->cl_dirty_transit,
+		CERROR("dirty %lu > dirty_max %lu\n",
+		       cli->cl_dirty_pages,
 		       cli->cl_dirty_max_pages);
 		oa->o_undirty = 0;
-	} else if (unlikely(atomic_long_read(&obd_dirty_pages) -
-			    atomic_long_read(&obd_dirty_transit_pages) >
+	} else if (unlikely(atomic_long_read(&obd_dirty_pages) >
 			    (long)(obd_max_dirty_pages + 1))) {
 		/* The atomic_read() allowing the atomic_inc() are
 		 * not covered by a lock thus they may safely race and trip
 		 * this CERROR() unless we add in a small fudge factor (+1).
 		 */
-		CERROR("%s: dirty %ld + %ld > system dirty_max %ld\n",
+		CERROR("%s: dirty %ld > system dirty_max %ld\n",
 		       cli_name(cli), atomic_long_read(&obd_dirty_pages),
-		       atomic_long_read(&obd_dirty_transit_pages),
 		       obd_max_dirty_pages);
 		oa->o_undirty = 0;
 	} else if (unlikely(cli->cl_dirty_max_pages - cli->cl_dirty_pages >
@@ -1047,7 +1045,7 @@ static int check_write_rcs(struct ptlrpc_request *req,
 static inline int can_merge_pages(struct brw_page *p1, struct brw_page *p2)
 {
 	if (p1->flag != p2->flag) {
-		unsigned int mask = ~(OBD_BRW_FROM_GRANT | OBD_BRW_NOCACHE |
+		unsigned int mask = ~(OBD_BRW_FROM_GRANT |
 				      OBD_BRW_SYNC | OBD_BRW_ASYNC |
 				      OBD_BRW_NOQUOTA | OBD_BRW_SOFT_SYNC);
 
diff --git a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
index 639db24b4f63..c2f49a3e0454 100644
--- a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
+++ b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c
@@ -1823,8 +1823,6 @@ void lustre_assert_wire_constants(void)
 		 OBD_BRW_FROM_GRANT);
 	LASSERTF(OBD_BRW_GRANTED == 0x40, "found 0x%.8x\n",
 		 OBD_BRW_GRANTED);
-	LASSERTF(OBD_BRW_NOCACHE == 0x80, "found 0x%.8x\n",
-		 OBD_BRW_NOCACHE);
 	LASSERTF(OBD_BRW_NOQUOTA == 0x100, "found 0x%.8x\n",
 		 OBD_BRW_NOQUOTA);
 	LASSERTF(OBD_BRW_SRVLOCK == 0x200, "found 0x%.8x\n",
-- 
2.14.0.rc0.dirty

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20190111/29144464/attachment.sig>
