[lustre-devel] [PATCH 23/29] lustre: osc_cache: remove 'transient' arg from osc_enter_cache_try
NeilBrown
neilb at suse.com
Thu Jan 10 16:27:38 PST 2019
On Thu, Jan 10 2019, Andreas Dilger wrote:
> 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>
Below is the revised version of this patch.
Thanks,
NeilBrown
From: NeilBrown <neilb at 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 at whamcloud.com>
Signed-off-by: NeilBrown <neilb at 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>
More information about the lustre-devel
mailing list