[lustre-devel] [PATCH 01/42] lustre: osc: pack osc_async_page better
James Simmons
jsimmons at infradead.org
Mon Jan 23 15:00:14 PST 2023
From: Patrick Farrell <pfarrell at whamcloud.com>
The oap_cmd field was used to store a number of other flags, but
those were redundant with oap_brw_page.flag, and never used.
That allows shrinking oap_cmd down to 2 bits.
Modern GCC allows specifying a bitfield for an enum, so the size
can be explicitly set.
The oap_page_off always holds < PAGE_SIZE, so it can safely fit
into PAGE_SHIFT bits, similar to ops_from. However, since this
field is used in math operations and we don't need the space,
always allocate it as an aligned 16-bit field.
This allows packing oap_async_flags, oap_cmd, and oap_page_off
into a 32-bit space. This avoids having holes in the struct. The
explicit oap_padding fields are needed so that "packed" does not
cause the fields to be misaligned, but still allows packing with
the following 4-byte field in osc_page.
Also move oap_brw_page to the end of the struct, since the
bp_padding field therein is useless and can be removed. This
allows better packing with the bitfields in struct osc_page.
brw_page old size: 32, holes: 0, padding: 4
brw_page new size: 28, holes: 0, padding: 0
osc_async_page old size: 104, holes: 8, padding: 4
osc_async_page new size: 92, holes: 0, bit holes: 10
osc_page old size: 144, holes: 8, bit holes: 4
osc_page new size: 128, holes: 0, bit holes: 4
Together this saves 16 bytes *per page* in cache,
and fits osc_page into a noce-sized allocation.
That is 512MiB on a system with 128GiB of cache.
WC-bug-id: https://jira.whamcloud.com/browse/LU-15619
Lustre-commit: 0bfc8eca5c3d26235 ("LU-15619 osc: pack osc_async_page better")
Signed-off-by: Patrick Farrell <pfarrell at whamcloud.com>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/46721
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Reviewed-by: Arshad Hussain <arshad.hussain at aeoncomputing.com>
Reviewed-by: Andreas Dilger <adilger at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
fs/lustre/include/lustre_osc.h | 25 +++++++++++++++++--------
fs/lustre/include/obd.h | 3 +--
fs/lustre/osc/osc_io.c | 5 ++---
fs/lustre/osc/osc_page.c | 4 +---
4 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/fs/lustre/include/lustre_osc.h b/fs/lustre/include/lustre_osc.h
index d15f46b4a34a..526093ebff18 100644
--- a/fs/lustre/include/lustre_osc.h
+++ b/fs/lustre/include/lustre_osc.h
@@ -60,7 +60,7 @@ struct osc_quota_info {
struct rcu_head rcu;
};
-enum async_flags {
+enum oap_async_flags {
ASYNC_READY = 0x1, /* osc_make_ready will not be called
* before this page is added to an rpc
*/
@@ -71,24 +71,32 @@ enum async_flags {
* to give the caller a chance to update
* or cancel the size of the io
*/
- ASYNC_HP = 0x10,
+ ASYNC_HP = 0x8,
+ OAP_ASYNC_MAX,
+ OAP_ASYNC_BITS = 4
};
+/* add explicit padding to keep fields aligned despite "packed",
+ * which is needed to pack with following field in osc_page
+ */
+#define OAP_PAD_BITS (16 - OBD_BRW_WRITE - OAP_ASYNC_BITS)
struct osc_async_page {
- unsigned short oap_cmd;
+ unsigned short oap_page_off; /* :PAGE_SHIFT */
+ unsigned int oap_cmd:OBD_BRW_WRITE;
+ enum oap_async_flags oap_async_flags:OAP_ASYNC_BITS;
+ unsigned int oap_padding1:OAP_PAD_BITS; /* unused */
+ unsigned int oap_padding2; /* unused */
struct list_head oap_pending_item;
struct list_head oap_rpc_item;
u64 oap_obj_off;
- unsigned int oap_page_off;
- enum async_flags oap_async_flags;
-
- struct brw_page oap_brw_page;
struct ptlrpc_request *oap_request;
struct osc_object *oap_obj;
-};
+
+ struct brw_page oap_brw_page;
+} __packed;
#define oap_page oap_brw_page.pg
#define oap_count oap_brw_page.count
@@ -96,6 +104,7 @@ struct osc_async_page {
static inline struct osc_async_page *brw_page2oap(struct brw_page *pga)
{
+ BUILD_BUG_ON(OAP_ASYNC_MAX - 1 >= (1 << OAP_ASYNC_BITS));
return container_of(pga, struct osc_async_page, oap_brw_page);
}
diff --git a/fs/lustre/include/obd.h b/fs/lustre/include/obd.h
index 56e56414fd72..e9752a306294 100644
--- a/fs/lustre/include/obd.h
+++ b/fs/lustre/include/obd.h
@@ -123,8 +123,7 @@ struct brw_page {
u16 bp_off_diff;
/* used for encryption: difference with count in clear text page */
u16 bp_count_diff;
- u32 bp_padding;
-};
+} __packed;
struct timeout_item {
enum timeout_event ti_event;
diff --git a/fs/lustre/osc/osc_io.c b/fs/lustre/osc/osc_io.c
index b9362d96b78d..c9a317575993 100644
--- a/fs/lustre/osc/osc_io.c
+++ b/fs/lustre/osc/osc_io.c
@@ -514,9 +514,8 @@ static bool trunc_check_cb(const struct lu_env *env, struct cl_io *io,
start, current->comm);
if (PageLocked(page->cp_vmpage))
- CDEBUG(D_CACHE, "page %p index %lu locked for %d.\n",
- ops, osc_index(ops),
- oap->oap_cmd & OBD_BRW_RWMASK);
+ CDEBUG(D_CACHE, "page %p index %lu locked for cmd=%d\n",
+ ops, osc_index(ops), oap->oap_cmd);
}
return true;
}
diff --git a/fs/lustre/osc/osc_page.c b/fs/lustre/osc/osc_page.c
index 667825a90442..feec99fe0ca2 100644
--- a/fs/lustre/osc/osc_page.c
+++ b/fs/lustre/osc/osc_page.c
@@ -296,10 +296,8 @@ void osc_page_submit(const struct lu_env *env, struct osc_page *opg,
oap->oap_count = opg->ops_to - opg->ops_from + 1;
oap->oap_brw_flags = OBD_BRW_SYNC | brw_flags;
- if (oio->oi_cap_sys_resource) {
+ if (oio->oi_cap_sys_resource)
oap->oap_brw_flags |= OBD_BRW_SYS_RESOURCE;
- oap->oap_cmd |= OBD_BRW_SYS_RESOURCE;
- }
osc_page_transfer_get(opg, "transfer\0imm");
osc_page_transfer_add(env, opg, crt);
--
2.27.0
More information about the lustre-devel
mailing list