All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Pavel Emelyanov <xemul@parallels.com>,
	James Bottomley <jbottomley@parallels.com>,
	Matthew Helsley <matt.helsley@gmail.com>,
	aneesh.kumar@linux.vnet.ibm.com, bfields@fieldses.org
Subject: Re: [patch 2/9] procfs: Convert /proc/pid/fdinfo/ handling routines to seq-file v2
Date: Sun, 26 Aug 2012 16:05:20 +0100	[thread overview]
Message-ID: <20120826150520.GZ23464@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20120826142820.GA16038@moon>

On Sun, Aug 26, 2012 at 06:28:20PM +0400, Cyrill Gorcunov wrote:
> On Sun, Aug 26, 2012 at 03:46:53AM +0100, Al Viro wrote:
> > 
> > IMO doing that at open() time is just a headache for no good reason -
> > resolving descriptor to struct file * at read() time as we do now
> > is much saner.  Better do that in your ->show(), since you are using
> > a single-shot iterator anyway...
> 
> Al, the updated version is below. I suppose I can grab proc inode then
> and lookup for file position and flags at show method. (I remember
> what you've said about O_CLOEXEC bit, but I'll address this in
> another patch).

Applied, with a couple of changes:
	* there's no need for those games with ihold/iput - opened file pins
its inode down just fine, TYVM.
	* struct fd_info is pointless in that form - the last argument
of single_open() will end up in seq_file ->private, so let's just pass
the inode there and use m->private in ->show().

I'll push that into vfs.git#master (along with the previous patch) in a few
hours.

O_CLOEXEC is taken care of in my tree.  FWIW, I'm consolidating descriptor
handling in general into fs/file.c (and I'm seriously tempted to rename
that sucker to something like fs/descriptors.c or fs/fdtable.c); some of
that stuff is already in #master.  I'm probably going to move some of the
code from your fs/proc/fd.c there as well...

  reply	other threads:[~2012-08-26 15:05 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-23 10:43 [patch 0/9] extended fdinfo via procfs series, v7 Cyrill Gorcunov
2012-08-23 10:43 ` [patch 1/9] procfs: Move /proc/pid/fd[info] handling code to fd.[ch] Cyrill Gorcunov
2012-08-25 17:16   ` Al Viro
2012-08-25 17:39     ` Cyrill Gorcunov
2012-08-25 17:55       ` Al Viro
2012-08-25 18:58         ` Cyrill Gorcunov
2012-08-25 19:12           ` Al Viro
2012-08-25 19:43             ` Cyrill Gorcunov
2012-08-25 21:52               ` Al Viro
2012-08-25 23:19     ` Al Viro
2012-08-23 10:43 ` [patch 2/9] procfs: Convert /proc/pid/fdinfo/ handling routines to seq-file v2 Cyrill Gorcunov
2012-08-26  2:46   ` Al Viro
2012-08-26  8:13     ` Cyrill Gorcunov
2012-08-26 14:28     ` Cyrill Gorcunov
2012-08-26 15:05       ` Al Viro [this message]
2012-08-26 15:10         ` Cyrill Gorcunov
2012-08-23 10:43 ` [patch 3/9] procfs: Add ability to plug in auxiliary fdinfo providers Cyrill Gorcunov
2012-08-23 10:43 ` [patch 4/9] fs, exportfs: Fix nil dereference if no s_export_op present Cyrill Gorcunov
2012-08-23 12:12   ` J. Bruce Fields
2012-08-23 12:34     ` Cyrill Gorcunov
2012-08-23 15:22       ` J. Bruce Fields
2012-08-23 10:43 ` [patch 5/9] fs, notify: Add file handle entry into inotify_inode_mark Cyrill Gorcunov
2012-08-23 10:43 ` [patch 6/9] fs, notify: Add procfs fdinfo helper v4 Cyrill Gorcunov
2012-08-23 10:43 ` [patch 7/9] fs, eventfd: Add procfs fdinfo helper Cyrill Gorcunov
2012-08-23 10:43 ` [patch 8/9] fs, epoll: Add procfs fdinfo helper v2 Cyrill Gorcunov
2012-08-23 10:43 ` [patch 9/9] fdinfo: Show sigmask for signalfd fd v2 Cyrill Gorcunov
2012-08-23 12:23 ` [patch 0/9] extended fdinfo via procfs series, v7 J. Bruce Fields
2012-08-23 12:44   ` Cyrill Gorcunov
2012-08-23 13:52     ` J. Bruce Fields
2012-08-23 13:56       ` Cyrill Gorcunov
2012-08-23 15:25         ` J. Bruce Fields
2012-08-23 17:02           ` Cyrill Gorcunov
2012-08-23 17:59             ` J. Bruce Fields
2012-08-23 18:03               ` Cyrill Gorcunov
2012-08-23 17:28     ` Cyrill Gorcunov

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=20120826150520.GZ23464@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=bfields@fieldses.org \
    --cc=gorcunov@openvz.org \
    --cc=jbottomley@parallels.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt.helsley@gmail.com \
    --cc=xemul@parallels.com \
    /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.