| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <PH0PR21MB302516C5334076716966B7EED7EE9@PH0PR21MB3025.namprd21.prod.outlook.com>
Date: Fri, 15 Apr 2022 03:34:06 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: "Andrea Parri (Microsoft)" <parri.andrea@...il.com>,
KY Srinivasan <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
Stefano Garzarella <sgarzare@...hat.com>,
David Miller <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 6/6] Drivers: hv: vmbus: Refactor the ring-buffer
iterator functions
From: Andrea Parri (Microsoft) <parri.andrea@...il.com> Sent: Wednesday, April 13, 2022 1:48 PM
>
> With no users of hv_pkt_iter_next_raw() and no "external" users of
> hv_pkt_iter_first_raw(), the iterator functions can be refactored
> and simplified to remove some indirection/code.
>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@...il.com>
> ---
> drivers/hv/ring_buffer.c | 11 +++++------
> include/linux/hyperv.h | 35 ++++-------------------------------
> 2 files changed, 9 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 3d215d9dec433..c9357dae2a2c8 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -421,7 +421,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
> memcpy(buffer, (const char *)desc + offset, packetlen);
>
> /* Advance ring index to next packet descriptor */
> - __hv_pkt_iter_next(channel, desc, true);
> + __hv_pkt_iter_next(channel, desc);
>
> /* Notify host of update */
> hv_pkt_iter_close(channel);
> @@ -459,7 +459,8 @@ static u32 hv_pkt_iter_avail(const struct hv_ring_buffer_info
> *rbi)
> /*
> * Get first vmbus packet without copying it out of the ring buffer
> */
> -struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct vmbus_channel *channel)
> +static struct vmpacket_descriptor *
> +hv_pkt_iter_first_raw(struct vmbus_channel *channel)
> {
> struct hv_ring_buffer_info *rbi = &channel->inbound;
>
> @@ -470,7 +471,6 @@ struct vmpacket_descriptor *hv_pkt_iter_first_raw(struct
> vmbus_channel *channel)
>
> return (struct vmpacket_descriptor *)(hv_get_ring_buffer(rbi) + rbi-
> >priv_read_index);
> }
> -EXPORT_SYMBOL_GPL(hv_pkt_iter_first_raw);
Does hv_pkt_iter_first_raw() need to be retained at all as a
separate function? I think after these changes, the only caller
is hv_pkt_iter_first(), in which case the code could just go
inline in hv_pkt_iter_first(). Doing that combining would
also allow the elimination of the duplicate call to
hv_pkt_iter_avail().
Michael
>
> /*
> * Get first vmbus packet from ring buffer after read_index
> @@ -534,8 +534,7 @@ EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
> */
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *desc,
> - bool copy)
> + const struct vmpacket_descriptor *desc)
> {
> struct hv_ring_buffer_info *rbi = &channel->inbound;
> u32 packetlen = desc->len8 << 3;
> @@ -548,7 +547,7 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
> rbi->priv_read_index -= dsize;
>
> /* more data? */
> - return copy ? hv_pkt_iter_first(channel) : hv_pkt_iter_first_raw(channel);
> + return hv_pkt_iter_first(channel);
> }
> EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
>
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 1112c5cf894e6..370adc9971d3e 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1673,55 +1673,28 @@ static inline u32 hv_pkt_len(const struct
> vmpacket_descriptor *desc)
> return desc->len8 << 3;
> }
>
> -struct vmpacket_descriptor *
> -hv_pkt_iter_first_raw(struct vmbus_channel *channel);
> -
> struct vmpacket_descriptor *
> hv_pkt_iter_first(struct vmbus_channel *channel);
>
> struct vmpacket_descriptor *
> __hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy);
> + const struct vmpacket_descriptor *pkt);
>
> void hv_pkt_iter_close(struct vmbus_channel *channel);
>
> static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_pkt(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt,
> - bool copy)
> +hv_pkt_iter_next(struct vmbus_channel *channel,
> + const struct vmpacket_descriptor *pkt)
> {
> struct vmpacket_descriptor *nxt;
>
> - nxt = __hv_pkt_iter_next(channel, pkt, copy);
> + nxt = __hv_pkt_iter_next(channel, pkt);
> if (!nxt)
> hv_pkt_iter_close(channel);
>
> return nxt;
> }
>
> -/*
> - * Get next packet descriptor without copying it out of the ring buffer
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next_raw(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, false);
> -}
> -
> -/*
> - * Get next packet descriptor from iterator
> - * If at end of list, return NULL and update host.
> - */
> -static inline struct vmpacket_descriptor *
> -hv_pkt_iter_next(struct vmbus_channel *channel,
> - const struct vmpacket_descriptor *pkt)
> -{
> - return hv_pkt_iter_next_pkt(channel, pkt, true);
> -}
> -
> #define foreach_vmbus_pkt(pkt, channel) \
> for (pkt = hv_pkt_iter_first(channel); pkt; \
> pkt = hv_pkt_iter_next(channel, pkt))
> --
> 2.25.1
Powered by blists - more mailing lists