All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: David Miller <davem@davemloft.net>
Cc: hch@lst.de, netdev@vger.kernel.org
Subject: Re: [PATCH] net: remove sock_iocb
Date: Thu, 29 Jan 2015 07:57:21 +0000	[thread overview]
Message-ID: <20150129075721.GD29656@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20150128.232211.1157376728217761804.davem@davemloft.net>

On Wed, Jan 28, 2015 at 11:22:11PM -0800, David Miller wrote:
> From: Christoph Hellwig <hch@lst.de>
> Date: Wed, 28 Jan 2015 18:04:53 +0100
> 
> > The sock_iocb structure is allocate on stack for each read/write-like
> > operation on sockets, and contains various fields of which only the
> > embedded msghdr and sometimes a pointer to the scm_cookie is ever used.
> > Get rid of the sock_iocb and put a msghdr directly on the stack and pass
> > the scm_cookie explicitly to netlink_mmap_sendmsg.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks good, applied, thanks.

You know, that's getting _really_ interesting.  The thing is, now
there's only one ->sendmsg() instance using iocb argument at all,
and it's a really weird one.  TIPC.  Which only compares it with
NULL, and that - to tell the normal calls (== done by sock_sendmsg()
et.al.) from tipc_{accept,connect}()-generated ones.  And the way
it's used is
        if (iocb)
                lock_sock(sk);
in tipc_send_stream().  IOW, "tipc_accept() and tipc_connect() would like
to use the guts of tipc_send_stream(), but they are already holding the
socket locked; let's just pass NULL iocb (which net/socket.c never does)
to tell it to leave the fucking lock alone, thank you very much".

And no ->recvmsg() are using iocb at all now.  How about we take the
guts of tipc_send_stream() into a helper function and have tipc_accept/connect
use _that_?  Then we could drop iocb argument completely and for ->sendmsg()
it would be the difference between 4 and 3 arguments, which has interesting
effects on certain register-starved architectures...

While we are at it, size (both for sendmsg and recvmsg) is always equal to
iov_iter_count(&msg->msg_iter), so that's not the only redundant argument
there...

Comments?

  reply	other threads:[~2015-01-29  7:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-28 17:04 [PATCH] net: remove sock_iocb Christoph Hellwig
2015-01-29  7:22 ` David Miller
2015-01-29  7:57   ` Al Viro [this message]
2015-01-29  8:01     ` David Miller
2015-01-29 12:13       ` Jon Maloy
2015-02-04  7:29     ` Ying Xue

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150129075721.GD29656@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=davem@davemloft.net \
    --cc=hch@lst.de \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.