On Thu, 20 Feb 2025, Matthew Sakai wrote:
> From: Ken Raeburn <raeb...@redhat.com>
>
> Reorder fields and make uds_request_type and uds_zone_message packed,
> to squeeze out some space. Use struct_group so the request reset code
> no longer needs to care about field order.
>
> On x86_64 this reduces the struct size from 144 to 120, which saves 48
> kB (about 12%) per VDO hash zone.
>
> Signed-off-by: Ken Raeburn <raeb...@redhat.com>
> Signed-off-by: Matthew Sakai <msa...@redhat.com>
Hi
This patch doesn't apply due to conflicts. Please resend this patch
against
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git
branch remotes/origin/for-next
I accepted the other 3 patches.
Mikulas
> ---
> drivers/md/dm-vdo/indexer/index-session.c | 6 +--
> drivers/md/dm-vdo/indexer/indexer.h | 53 +++++++++++------------
> 2 files changed, 27 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/md/dm-vdo/indexer/index-session.c
> b/drivers/md/dm-vdo/indexer/index-session.c
> index aee0914d604a..aa575a24e0b2 100644
> --- a/drivers/md/dm-vdo/indexer/index-session.c
> +++ b/drivers/md/dm-vdo/indexer/index-session.c
> @@ -100,7 +100,6 @@ static int get_index_session(struct uds_index_session
> *index_session)
>
> int uds_launch_request(struct uds_request *request)
> {
> - size_t internal_size;
> int result;
>
> if (request->callback == NULL) {
> @@ -121,10 +120,7 @@ int uds_launch_request(struct uds_request *request)
> }
>
> /* Reset all internal fields before processing. */
> - internal_size =
> - sizeof(struct uds_request) - offsetof(struct uds_request,
> zone_number);
> - // FIXME should be using struct_group for this instead
> - memset((char *) request + sizeof(*request) - internal_size, 0,
> internal_size);
> + memset(&request->internal, 0, sizeof(request->internal));
>
> result = get_index_session(request->session);
> if (result != UDS_SUCCESS)
> diff --git a/drivers/md/dm-vdo/indexer/indexer.h
> b/drivers/md/dm-vdo/indexer/indexer.h
> index 3744aaf625b0..d765f24328eb 100644
> --- a/drivers/md/dm-vdo/indexer/indexer.h
> +++ b/drivers/md/dm-vdo/indexer/indexer.h
> @@ -8,6 +8,7 @@
>
> #include <linux/mutex.h>
> #include <linux/sched.h>
> +#include <linux/stddef.h>
> #include <linux/types.h>
> #include <linux/wait.h>
>
> @@ -73,7 +74,7 @@ enum uds_request_type {
> /* Remove any mapping for a name. */
> UDS_DELETE,
>
> -};
> +} __packed;
>
> enum uds_open_index_type {
> /* Create a new index. */
> @@ -226,7 +227,7 @@ struct uds_zone_message {
> enum uds_zone_message_type type;
> /* The virtual chapter number to which the message applies */
> u64 virtual_chapter;
> -};
> +} __packed;
>
> struct uds_index_session;
> struct uds_index;
> @@ -253,34 +254,32 @@ struct uds_request {
>
> /* The existing data associated with the request name, if any */
> struct uds_record_data old_metadata;
> - /* Either UDS_SUCCESS or an error code for the request */
> - int status;
> /* True if the record name had an existing entry in the index */
> bool found;
> + /* Either UDS_SUCCESS or an error code for the request */
> + int status;
>
> - /*
> - * The remaining fields are used internally and should not be altered
> by clients. The index
> - * relies on zone_number being the first field in this section.
> - */
> -
> - /* The number of the zone which will process this request*/
> - unsigned int zone_number;
> - /* A link for adding a request to a lock-free queue */
> - struct funnel_queue_entry queue_link;
> - /* A link for adding a request to a standard linked list */
> - struct uds_request *next_request;
> - /* A pointer to the index processing this request */
> - struct uds_index *index;
> - /* Control message for coordinating between zones */
> - struct uds_zone_message zone_message;
> - /* If true, process request immediately by waking the worker thread */
> - bool unbatched;
> - /* If true, continue this request before processing newer requests */
> - bool requeued;
> - /* The virtual chapter containing the record name, if known */
> - u64 virtual_chapter;
> - /* The region of the index containing the record name */
> - enum uds_index_region location;
> + /* The remaining fields are used internally and should not be altered
> by clients. */
> + struct_group(internal,
> + /* The virtual chapter containing the record name, if
> known */
> + u64 virtual_chapter;
> + /* The region of the index containing the record name */
> + enum uds_index_region location;
> + /* If true, process request immediately by waking the
> worker thread */
> + bool unbatched;
> + /* If true, continue this request before processing newer
> requests */
> + bool requeued;
> + /* Control message for coordinating between zones */
> + struct uds_zone_message zone_message;
> + /* The number of the zone which will process this request*/
> + unsigned int zone_number;
> + /* A link for adding a request to a lock-free queue */
> + struct funnel_queue_entry queue_link;
> + /* A link for adding a request to a standard linked list */
> + struct uds_request *next_request;
> + /* A pointer to the index processing this request */
> + struct uds_index *index;
> + );
> };
>
> /* Compute the number of bytes needed to store an index. */
> --
> 2.45.2
>
>
