lkml.org 
[lkml]   [2017]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 09/13] xen/pvcalls: implement sendmsg
From
Date

> +static bool pvcalls_front_write_todo(struct sock_mapping *map)
> +{
> + struct pvcalls_data_intf *intf = map->active.ring;
> + RING_IDX cons, prod, size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
> + int32_t error;
> +
> + cons = intf->out_cons;
> + prod = intf->out_prod;
> + error = intf->out_error;
> + if (error == -ENOTCONN)
> + return false;
> + if (error != 0)
> + return true;

Just like below, error processing can be moved up.

> + return !!(size - pvcalls_queued(prod, cons, size));
> +}
> +
> static irqreturn_t pvcalls_front_event_handler(int irq, void *dev_id)
> {
> struct xenbus_device *dev = dev_id;
> @@ -363,6 +380,108 @@ int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr,
> return ret;
> }
>
> +static int __write_ring(struct pvcalls_data_intf *intf,
> + struct pvcalls_data *data,
> + struct iov_iter *msg_iter,
> + int len)
> +{
> + RING_IDX cons, prod, size, masked_prod, masked_cons;
> + RING_IDX array_size = XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
> + int32_t error;
> +
> + error = intf->out_error;
> + if (error < 0)
> + return error;
> + cons = intf->out_cons;
> + prod = intf->out_prod;
> + /* read indexes before continuing */
> + virt_mb();
> +
> + size = pvcalls_queued(prod, cons, array_size);
> + if (size >= array_size)
> + return 0;


Is it possible to have size > array_size?


> + if (len > array_size - size)
> + len = array_size - size;
> +
> + masked_prod = pvcalls_mask(prod, array_size);
> + masked_cons = pvcalls_mask(cons, array_size);
> +
> + if (masked_prod < masked_cons) {
> + copy_from_iter(data->out + masked_prod, len, msg_iter);
> + } else {
> + if (len > array_size - masked_prod) {
> + copy_from_iter(data->out + masked_prod,
> + array_size - masked_prod, msg_iter);
> + copy_from_iter(data->out,
> + len - (array_size - masked_prod),
> + msg_iter);
> + } else {
> + copy_from_iter(data->out + masked_prod, len, msg_iter);
> + }
> + }
> + /* write to ring before updating pointer */
> + virt_wmb();
> + intf->out_prod += len;
> +
> + return len;


I know that you said you'd be changing len's type to int but now that I
am looking at it I wonder whether you could pass len as a 'size_t *' and
have this routine return error code (i.e. <=0).

OTOH, we'd be mixing up types again since RING_IDX is an unsigned int.

So I'll leave it to you (or anyone else reviewing this) to decide which
way is better.


> +}
> +
> +int pvcalls_front_sendmsg(struct socket *sock, struct msghdr *msg,
> + size_t len)

Also, the signature here looks suspicious --- you are trying to send
'size_t len' bytes but returning an int, which is how many bytes you've
actually sent. Right?


-boris

\
 
 \ /
  Last update: 2017-09-22 23:58    [W:0.168 / U:6.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site