Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1432688 > unrolled thread

Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets David Miller <davem@davemloft.net> - 2016-06-28 11:40 +0200
    RE: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-06-28 13:00 +0200
      Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets David Miller <davem@davemloft.net> - 2016-06-28 15:50 +0200
        RE: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-06-28 17:40 +0200
      Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets Rick Jones <rick.jones2@hpe.com> - 2016-06-28 17:50 +0200
        RE: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets Dexuan Cui <decui@microsoft.com> - 2016-06-29 11:20 +0200

#1432688 — Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets

From: Dexuan Cui <decui@microsoft.com>
Date: Fri, 24 Jun 2016 07:45:24 +0000

> +	while ((ret = vmalloc(size)) == NULL)
> +		ssleep(1);

This is completely, and entirely, unacceptable.

If the allocation fails, you return an error and release
your resources.

You don't just loop forever waiting for it to succeed.

[toc] | [next] | [standalone]

#1432846

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, June 28, 2016 17:34
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; vkuznets@redhat.com;
> cavery@redhat.com; KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; joe@perches.com
> Subject: Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets
> 
> From: Dexuan Cui <decui@microsoft.com>
> Date: Fri, 24 Jun 2016 07:45:24 +0000
> 
> > +	while ((ret = vmalloc(size)) == NULL)
> > +		ssleep(1);
> 
> This is completely, and entirely, unacceptable.
> 
> If the allocation fails, you return an error and release
> your resources.
> 
> You don't just loop forever waiting for it to succeed.

Hi David,
I agree this is ugly...

The idea here is: IMO the syscalls sys_read()/write() shoudn't return
-ENOMEM, so I have to make sure the buffer allocation succeeds?

I tried to use kmalloc with __GFP_NOFAIL, but I hit a warning in 
in mm/page_alloc.c:
WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));

What error code do you think I should return? 
EAGAIN, ERESTARTSYS, or something else?

May I have your suggestion? Thanks!

-- Dexuan

[toc] | [prev] | [next] | [standalone]

#1432998

From: Dexuan Cui <decui@microsoft.com>
Date: Tue, 28 Jun 2016 09:59:21 +0000

> The idea here is: IMO the syscalls sys_read()/write() shoudn't return
> -ENOMEM, so I have to make sure the buffer allocation succeeds?

You have to fail if resources cannot be allocated.

[toc] | [prev] | [next] | [standalone]

#1433068

> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, June 28, 2016 21:45
> To: Dexuan Cui <decui@microsoft.com>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; vkuznets@redhat.com;
> cavery@redhat.com; KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; joe@perches.com
> Subject: Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets
> 
> From: Dexuan Cui <decui@microsoft.com>
> Date: Tue, 28 Jun 2016 09:59:21 +0000
> 
> > The idea here is: IMO the syscalls sys_read()/write() shoudn't return
> > -ENOMEM, so I have to make sure the buffer allocation succeeds?
> 
> You have to fail if resources cannot be allocated.

OK, I'll try to fix this, probably by returning -EAGAIN or -ERESTARTSYS.

I'll report back ASAP.

Thanks,
-- Dexuan

[toc] | [prev] | [next] | [standalone]

#1433074

On 06/28/2016 02:59 AM, Dexuan Cui wrote:
> The idea here is: IMO the syscalls sys_read()/write() shoudn't return
> -ENOMEM, so I have to make sure the buffer allocation succeeds?
>
> I tried to use kmalloc with __GFP_NOFAIL, but I hit a warning in
> in mm/page_alloc.c:
> WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));
>
> What error code do you think I should return?
> EAGAIN, ERESTARTSYS, or something else?
>
> May I have your suggestion? Thanks!

What happens as far as errno is concerned when an application makes a 
read() call against a (say TCP) socket associated with a connection 
which has been reset?  Is it limited to those errno values listed in the 
read() manpage, or does it end-up getting an errno value from those 
listed in the recv() manpage?  Or, perhaps even one not (presently) 
listed in either?

rick jones

[toc] | [prev] | [next] | [standalone]

#1433596

> From: Rick Jones [mailto:rick.jones2@hpe.com]
> Sent: Tuesday, June 28, 2016 23:43
> To: Dexuan Cui <decui@microsoft.com>; David Miller <davem@davemloft.net>
> Cc: gregkh@linuxfoundation.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; devel@linuxdriverproject.org; olaf@aepfle.de;
> apw@canonical.com; jasowang@redhat.com; vkuznets@redhat.com;
> cavery@redhat.com; KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; joe@perches.com
> Subject: Re: [PATCH v12 net-next 1/1] hv_sock: introduce Hyper-V Sockets
> 
> On 06/28/2016 02:59 AM, Dexuan Cui wrote:
> > The idea here is: IMO the syscalls sys_read()/write() shoudn't return
> > -ENOMEM, so I have to make sure the buffer allocation succeeds?
> >
> > I tried to use kmalloc with __GFP_NOFAIL, but I hit a warning in
> > in mm/page_alloc.c:
> > WARN_ON_ONCE((gfp_flags & __GFP_NOFAIL) && (order > 1));
> >
> > What error code do you think I should return?
> > EAGAIN, ERESTARTSYS, or something else?
> >
> > May I have your suggestion? Thanks!
> 
> What happens as far as errno is concerned when an application makes a
> read() call against a (say TCP) socket associated with a connection
> which has been reset? 
I suppose it is ECONNRESET (Connection reset by peer).

>  Is it limited to those errno values listed in the
> read() manpage, or does it end-up getting an errno value from those
> listed in the recv() manpage?  Or, perhaps even one not (presently)
> listed in either?
> 
> rick jones

Actually "man read/write" says "Other errors may occur, depending on the
object connected to fd".

"man send/recv" indeed lists ENOMEM.

Considering AF_HYPERV is a new socket type, ENOMEM seems OK to me
and I'm going to post a new version of the patch.

In the long run, I think we should add a new API in the VMBus driver,
allowing data copy from VMBus ringbuffer into user mode buffer directly.
This way, we can even eliminate this temporary buffer.

Thanks,
-- Dexuan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web