Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Mon, Nov 24, 2014 at 12:27:42AM +, Ben Hutchings wrote: > > copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN; > > if (copylen > good_linear) > > copylen = good_linear; > > linear = copylen; > > - if (iov_pages(iv, vnet_hdr_len + copylen, count) > > - <= MAX_SKB_FRAGS) > > + i = *from; > > + iov_iter_advance(, copylen); > > + if (iov_iter_npages(, INT_MAX) <= MAX_SKB_FRAGS) > > The maxpages argument should be MAX_SKB_FRAGS + 1 as we don't need the > exact number. In principle, that's true, but... Do we really care? It only buys you anything if you have a monstrously fragmented iovec. And if you end up spending too considerable amount of time in that loop in iov_iter_npages, you are by definition on the slow path - it *will* fail, since we do not even try to merge adjacent iovec segments. Never had... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Mon, Nov 24, 2014 at 12:27:42AM +, Ben Hutchings wrote: copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN; if (copylen good_linear) copylen = good_linear; linear = copylen; - if (iov_pages(iv, vnet_hdr_len + copylen, count) - = MAX_SKB_FRAGS) + i = *from; + iov_iter_advance(i, copylen); + if (iov_iter_npages(i, INT_MAX) = MAX_SKB_FRAGS) The maxpages argument should be MAX_SKB_FRAGS + 1 as we don't need the exact number. In principle, that's true, but... Do we really care? It only buys you anything if you have a monstrously fragmented iovec. And if you end up spending too considerable amount of time in that loop in iov_iter_npages, you are by definition on the slow path - it *will* fail, since we do not even try to merge adjacent iovec segments. Never had... -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Mon, 2014-11-24 at 00:27 +, Ben Hutchings wrote: > On Sat, 2014-11-22 at 04:33 +, Al Viro wrote: [...] > Does skb_copy_datagram_from_iter() really need a len parameter? Here it > is equal to iov_iter_count(from). [...] > Again len is equal to iov_iter_count(from), so I think that parameter is > redundant. Having read further patches, I see that unix_stream_sendmsg() is the one exception where the length parameter is different. But maybe the common case (len = iter_iov_count(iov)) deserves a wrapper function? Ben. -- Ben Hutchings Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer signature.asc Description: This is a digitally signed message part
Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Sat, 2014-11-22 at 04:33 +, Al Viro wrote:
> allows to switch macvtap and tun from ->aio_write() to ->write_iter()
>
> Signed-off-by: Al Viro
> ---
> drivers/net/macvtap.c | 43 ---
> drivers/net/tun.c | 43 +++
> 2 files changed, 43 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index cdd820f..2bf08c6 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct
> sk_buff *skb,
>
> /* Get packet from user space buffer */
> static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
> - const struct iovec *iv, unsigned long total_len,
> - size_t count, int noblock)
> + struct iov_iter *from, int noblock)
> {
> int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
> struct sk_buff *skb;
> struct macvlan_dev *vlan;
> + unsigned long total_len = iov_iter_count(from);
> unsigned long len = total_len;
> int err;
> struct virtio_net_hdr vnet_hdr = { 0 };
> @@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
> struct msghdr *m,
> int copylen = 0;
> bool zerocopy = false;
> size_t linear;
> + ssize_t n;
>
> if (q->flags & IFF_VNET_HDR) {
> vnet_hdr_len = q->vnet_hdr_sz;
> @@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue
> *q, struct msghdr *m,
> goto err;
> len -= vnet_hdr_len;
>
> - err = memcpy_fromiovecend((void *)_hdr, iv, 0,
> -sizeof(vnet_hdr));
> - if (err < 0)
> + err = -EFAULT;
> + n = copy_from_iter(_hdr, sizeof(vnet_hdr), from);
> + if (n != sizeof(vnet_hdr))
> goto err;
> + iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
> if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
>vnet_hdr.csum_start + vnet_hdr.csum_offset + 2 >
> vnet_hdr.hdr_len)
> @@ -680,17 +682,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue
> *q, struct msghdr *m,
> if (unlikely(len < ETH_HLEN))
> goto err;
>
> - err = -EMSGSIZE;
> - if (unlikely(count > UIO_MAXIOV))
> - goto err;
> -
> if (m && m->msg_control && sock_flag(>sk, SOCK_ZEROCOPY)) {
> + struct iov_iter i;
Blank line needed after a declaration.
> copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
> if (copylen > good_linear)
> copylen = good_linear;
> linear = copylen;
> - if (iov_pages(iv, vnet_hdr_len + copylen, count)
> - <= MAX_SKB_FRAGS)
> + i = *from;
> + iov_iter_advance(, copylen);
> + if (iov_iter_npages(, INT_MAX) <= MAX_SKB_FRAGS)
The maxpages argument should be MAX_SKB_FRAGS + 1 as we don't need the
exact number.
> zerocopy = true;
> }
>
> @@ -708,10 +708,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
> struct msghdr *m,
> goto err;
>
> if (zerocopy)
> - err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
> + err = zerocopy_sg_from_iter(skb, from);
> else {
> - err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
> -len);
> + err = skb_copy_datagram_from_iter(skb, 0, from, len);
Does skb_copy_datagram_from_iter() really need a len parameter? Here it
is equal to iov_iter_count(from).
[...]
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1012,28 +1012,29 @@ static struct sk_buff *tun_alloc_skb(struct tun_file
> *tfile,
>
> /* Get packet from user space buffer */
> static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> - void *msg_control, const struct iovec *iv,
> - size_t total_len, size_t count, int noblock)
> + void *msg_control, struct iov_iter *from,
> + int noblock)
> {
> struct tun_pi pi = { 0, cpu_to_be16(ETH_P_IP) };
> struct sk_buff *skb;
> + size_t total_len = iov_iter_count(from);
> size_t len = total_len, align = NET_SKB_PAD, linear;
> struct virtio_net_hdr gso = { 0 };
> int good_linear;
> - int offset = 0;
> int copylen;
> bool zerocopy = false;
> int err;
> u32 rxhash;
> + ssize_t n;
>
> if (!(tun->flags & TUN_NO_PI)) {
> if (len < sizeof(pi))
> return -EINVAL;
> len -=
Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Sat, 2014-11-22 at 04:33 +, Al Viro wrote:
allows to switch macvtap and tun from -aio_write() to -write_iter()
Signed-off-by: Al Viro v...@zeniv.linux.org.uk
---
drivers/net/macvtap.c | 43 ---
drivers/net/tun.c | 43 +++
2 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index cdd820f..2bf08c6 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct
sk_buff *skb,
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
- const struct iovec *iv, unsigned long total_len,
- size_t count, int noblock)
+ struct iov_iter *from, int noblock)
{
int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
struct sk_buff *skb;
struct macvlan_dev *vlan;
+ unsigned long total_len = iov_iter_count(from);
unsigned long len = total_len;
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
@@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
int copylen = 0;
bool zerocopy = false;
size_t linear;
+ ssize_t n;
if (q-flags IFF_VNET_HDR) {
vnet_hdr_len = q-vnet_hdr_sz;
@@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue
*q, struct msghdr *m,
goto err;
len -= vnet_hdr_len;
- err = memcpy_fromiovecend((void *)vnet_hdr, iv, 0,
-sizeof(vnet_hdr));
- if (err 0)
+ err = -EFAULT;
+ n = copy_from_iter(vnet_hdr, sizeof(vnet_hdr), from);
+ if (n != sizeof(vnet_hdr))
goto err;
+ iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
if ((vnet_hdr.flags VIRTIO_NET_HDR_F_NEEDS_CSUM)
vnet_hdr.csum_start + vnet_hdr.csum_offset + 2
vnet_hdr.hdr_len)
@@ -680,17 +682,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue
*q, struct msghdr *m,
if (unlikely(len ETH_HLEN))
goto err;
- err = -EMSGSIZE;
- if (unlikely(count UIO_MAXIOV))
- goto err;
-
if (m m-msg_control sock_flag(q-sk, SOCK_ZEROCOPY)) {
+ struct iov_iter i;
Blank line needed after a declaration.
copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
if (copylen good_linear)
copylen = good_linear;
linear = copylen;
- if (iov_pages(iv, vnet_hdr_len + copylen, count)
- = MAX_SKB_FRAGS)
+ i = *from;
+ iov_iter_advance(i, copylen);
+ if (iov_iter_npages(i, INT_MAX) = MAX_SKB_FRAGS)
The maxpages argument should be MAX_SKB_FRAGS + 1 as we don't need the
exact number.
zerocopy = true;
}
@@ -708,10 +708,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
goto err;
if (zerocopy)
- err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+ err = zerocopy_sg_from_iter(skb, from);
else {
- err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
-len);
+ err = skb_copy_datagram_from_iter(skb, 0, from, len);
Does skb_copy_datagram_from_iter() really need a len parameter? Here it
is equal to iov_iter_count(from).
[...]
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1012,28 +1012,29 @@ static struct sk_buff *tun_alloc_skb(struct tun_file
*tfile,
/* Get packet from user space buffer */
static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
- void *msg_control, const struct iovec *iv,
- size_t total_len, size_t count, int noblock)
+ void *msg_control, struct iov_iter *from,
+ int noblock)
{
struct tun_pi pi = { 0, cpu_to_be16(ETH_P_IP) };
struct sk_buff *skb;
+ size_t total_len = iov_iter_count(from);
size_t len = total_len, align = NET_SKB_PAD, linear;
struct virtio_net_hdr gso = { 0 };
int good_linear;
- int offset = 0;
int copylen;
bool zerocopy = false;
int err;
u32 rxhash;
+ ssize_t n;
if (!(tun-flags TUN_NO_PI)) {
if (len sizeof(pi))
return -EINVAL;
len -= sizeof(pi);
- if (memcpy_fromiovecend((void *)pi, iv, 0, sizeof(pi)))
+ n =
Re: [PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
On Mon, 2014-11-24 at 00:27 +, Ben Hutchings wrote: On Sat, 2014-11-22 at 04:33 +, Al Viro wrote: [...] Does skb_copy_datagram_from_iter() really need a len parameter? Here it is equal to iov_iter_count(from). [...] Again len is equal to iov_iter_count(from), so I think that parameter is redundant. Having read further patches, I see that unix_stream_sendmsg() is the one exception where the length parameter is different. But maybe the common case (len = iter_iov_count(iov)) deserves a wrapper function? Ben. -- Ben Hutchings Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer signature.asc Description: This is a digitally signed message part
[PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
allows to switch macvtap and tun from ->aio_write() to ->write_iter()
Signed-off-by: Al Viro
---
drivers/net/macvtap.c | 43 ---
drivers/net/tun.c | 43 +++
2 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index cdd820f..2bf08c6 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff
*skb,
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
- const struct iovec *iv, unsigned long total_len,
- size_t count, int noblock)
+ struct iov_iter *from, int noblock)
{
int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
struct sk_buff *skb;
struct macvlan_dev *vlan;
+ unsigned long total_len = iov_iter_count(from);
unsigned long len = total_len;
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
@@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
int copylen = 0;
bool zerocopy = false;
size_t linear;
+ ssize_t n;
if (q->flags & IFF_VNET_HDR) {
vnet_hdr_len = q->vnet_hdr_sz;
@@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
goto err;
len -= vnet_hdr_len;
- err = memcpy_fromiovecend((void *)_hdr, iv, 0,
- sizeof(vnet_hdr));
- if (err < 0)
+ err = -EFAULT;
+ n = copy_from_iter(_hdr, sizeof(vnet_hdr), from);
+ if (n != sizeof(vnet_hdr))
goto err;
+ iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
vnet_hdr.csum_start + vnet_hdr.csum_offset + 2 >
vnet_hdr.hdr_len)
@@ -680,17 +682,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
if (unlikely(len < ETH_HLEN))
goto err;
- err = -EMSGSIZE;
- if (unlikely(count > UIO_MAXIOV))
- goto err;
-
if (m && m->msg_control && sock_flag(>sk, SOCK_ZEROCOPY)) {
+ struct iov_iter i;
copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
if (copylen > good_linear)
copylen = good_linear;
linear = copylen;
- if (iov_pages(iv, vnet_hdr_len + copylen, count)
- <= MAX_SKB_FRAGS)
+ i = *from;
+ iov_iter_advance(, copylen);
+ if (iov_iter_npages(, INT_MAX) <= MAX_SKB_FRAGS)
zerocopy = true;
}
@@ -708,10 +708,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
goto err;
if (zerocopy)
- err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+ err = zerocopy_sg_from_iter(skb, from);
else {
- err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
- len);
+ err = skb_copy_datagram_from_iter(skb, 0, from, len);
if (!err && m && m->msg_control) {
struct ubuf_info *uarg = m->msg_control;
uarg->callback(uarg, false);
@@ -764,16 +763,12 @@ err:
return err;
}
-static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
-unsigned long count, loff_t pos)
+static ssize_t macvtap_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
- ssize_t result = -ENOLINK;
struct macvtap_queue *q = file->private_data;
- result = macvtap_get_user(q, NULL, iv, iov_length(iv, count), count,
- file->f_flags & O_NONBLOCK);
- return result;
+ return macvtap_get_user(q, NULL, from, file->f_flags & O_NONBLOCK);
}
/* Put packet to the user space buffer */
@@ -1079,8 +1074,9 @@ static const struct file_operations macvtap_fops = {
.open = macvtap_open,
.release= macvtap_release,
.read = new_sync_read,
+ .write = new_sync_write,
.read_iter = macvtap_read_iter,
- .aio_write = macvtap_aio_write,
+ .write_iter = macvtap_write_iter,
.poll = macvtap_poll,
.llseek = no_llseek,
.unlocked_ioctl = macvtap_ioctl,
@@ -1093,8 +1089,9 @@ static int macvtap_sendmsg(struct kiocb *iocb, struct
[PATCH 08/17] {macvtap,tun}_get_user(): switch to iov_iter
allows to switch macvtap and tun from -aio_write() to -write_iter()
Signed-off-by: Al Viro v...@zeniv.linux.org.uk
---
drivers/net/macvtap.c | 43 ---
drivers/net/tun.c | 43 +++
2 files changed, 43 insertions(+), 43 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index cdd820f..2bf08c6 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -640,12 +640,12 @@ static void macvtap_skb_to_vnet_hdr(const struct sk_buff
*skb,
/* Get packet from user space buffer */
static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
- const struct iovec *iv, unsigned long total_len,
- size_t count, int noblock)
+ struct iov_iter *from, int noblock)
{
int good_linear = SKB_MAX_HEAD(NET_IP_ALIGN);
struct sk_buff *skb;
struct macvlan_dev *vlan;
+ unsigned long total_len = iov_iter_count(from);
unsigned long len = total_len;
int err;
struct virtio_net_hdr vnet_hdr = { 0 };
@@ -653,6 +653,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
int copylen = 0;
bool zerocopy = false;
size_t linear;
+ ssize_t n;
if (q-flags IFF_VNET_HDR) {
vnet_hdr_len = q-vnet_hdr_sz;
@@ -662,10 +663,11 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
goto err;
len -= vnet_hdr_len;
- err = memcpy_fromiovecend((void *)vnet_hdr, iv, 0,
- sizeof(vnet_hdr));
- if (err 0)
+ err = -EFAULT;
+ n = copy_from_iter(vnet_hdr, sizeof(vnet_hdr), from);
+ if (n != sizeof(vnet_hdr))
goto err;
+ iov_iter_advance(from, vnet_hdr_len - sizeof(vnet_hdr));
if ((vnet_hdr.flags VIRTIO_NET_HDR_F_NEEDS_CSUM)
vnet_hdr.csum_start + vnet_hdr.csum_offset + 2
vnet_hdr.hdr_len)
@@ -680,17 +682,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
if (unlikely(len ETH_HLEN))
goto err;
- err = -EMSGSIZE;
- if (unlikely(count UIO_MAXIOV))
- goto err;
-
if (m m-msg_control sock_flag(q-sk, SOCK_ZEROCOPY)) {
+ struct iov_iter i;
copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
if (copylen good_linear)
copylen = good_linear;
linear = copylen;
- if (iov_pages(iv, vnet_hdr_len + copylen, count)
- = MAX_SKB_FRAGS)
+ i = *from;
+ iov_iter_advance(i, copylen);
+ if (iov_iter_npages(i, INT_MAX) = MAX_SKB_FRAGS)
zerocopy = true;
}
@@ -708,10 +708,9 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
struct msghdr *m,
goto err;
if (zerocopy)
- err = zerocopy_sg_from_iovec(skb, iv, vnet_hdr_len, count);
+ err = zerocopy_sg_from_iter(skb, from);
else {
- err = skb_copy_datagram_from_iovec(skb, 0, iv, vnet_hdr_len,
- len);
+ err = skb_copy_datagram_from_iter(skb, 0, from, len);
if (!err m m-msg_control) {
struct ubuf_info *uarg = m-msg_control;
uarg-callback(uarg, false);
@@ -764,16 +763,12 @@ err:
return err;
}
-static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
-unsigned long count, loff_t pos)
+static ssize_t macvtap_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb-ki_filp;
- ssize_t result = -ENOLINK;
struct macvtap_queue *q = file-private_data;
- result = macvtap_get_user(q, NULL, iv, iov_length(iv, count), count,
- file-f_flags O_NONBLOCK);
- return result;
+ return macvtap_get_user(q, NULL, from, file-f_flags O_NONBLOCK);
}
/* Put packet to the user space buffer */
@@ -1079,8 +1074,9 @@ static const struct file_operations macvtap_fops = {
.open = macvtap_open,
.release= macvtap_release,
.read = new_sync_read,
+ .write = new_sync_write,
.read_iter = macvtap_read_iter,
- .aio_write = macvtap_aio_write,
+ .write_iter = macvtap_write_iter,
.poll = macvtap_poll,
.llseek = no_llseek,
.unlocked_ioctl = macvtap_ioctl,
@@ -1093,8 +1089,9 @@ static int macvtap_sendmsg(struct kiocb *iocb,
