All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	hch@infradead.org, torvalds@linux-foundation.org,
	dhowells@redhat.com, mszeredi@suse.cz
Subject: Re: [PATCH 00/21] vfs: atomic open v6 (part 2)
Date: Sun, 10 Jun 2012 12:10:56 +0100	[thread overview]
Message-ID: <20120610111056.GD30000@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20120610034921.GB30000@ZenIV.linux.org.uk>

On Sun, Jun 10, 2012 at 04:49:21AM +0100, Al Viro wrote:

> So the only remaining reason for having that thing is this: what if we
> call ->atomic_open(), but it doesn't call finish_open()?  Then we need
> to free that unused struct file.  If finish_open() failed, we wouldn't.
> Same if it succeeded and something *after* it in ->atomic_open() failed
> (then we need to fput() that file - your code in ceph leaks it, BTW).
> Fair enough.  So we need to add one more helper that would discard that
> half-set-up struct file as we want it to be discarded.  That's all.

Actually, I take that back - that code in ceph is unreachable when
finish_open() succeeds.

Anyway, see vfs.git#atomic_open; it's a port of your queue + COMPLETELY
UNTESTED followups massaging it along the following lines:
	* ->atomic_open() takes struct file * instead of struct opendata *
	* it return int instead of struct file * - 0 for succeess, -E...
for error, 1 for "here's your sodding dentry, do it yourself".  Said
dentry is returned via file->f_path.dentry.
	* the same had been done to atomic_open()/lookup_open()/do_last()
	* finish_open() takes struct file and returns an int
	* it *also* takes int * - used to keep track of whether we'd done
successful do_dentry_open(), instead of "has opendata->filp been cleared?"
as in your variant.  Said int * is what your bool *created of ->atomic_open()
and friends has been turned into.  So the check in path_openat() is
	if (!(opened & FILE_OPENED)) {
		BUG_ON(!error);
		put_filp(file);
	}
which is as explicit as it gets, IMO.

The forest of failure exits in do_last() got cleaned up a bit, BTW.  Probably
can be cleaned up some more...

WARNING: I haven't even tried to boot it.  It builds, but this is all I can
promise at the moment.  I'm about to fall down (it's 7am here already ;-/),
will give it some beating when I get up.  It almost certainly has bugs, so
consider that as call for review and not much more.

  parent reply	other threads:[~2012-06-10 11:10 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-05 13:10 [PATCH 00/21] vfs: atomic open v6 (part 2) Miklos Szeredi
2012-06-05 13:10 ` [PATCH 01/21] vfs: do_last(): inline lookup_slow() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 02/21] vfs: do_last(): separate O_CREAT specific code Miklos Szeredi
2012-06-05 13:10 ` [PATCH 03/21] vfs: do_last(): common slow lookup Miklos Szeredi
2012-06-05 13:10 ` [PATCH 04/21] vfs: add lookup_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 05/21] vfs: lookup_open(): expand lookup_hash() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 06/21] vfs: add i_op->atomic_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 07/21] nfs: implement i_op->atomic_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 08/21] nfs: clean up ->create in nfs_rpc_ops Miklos Szeredi
2012-06-05 13:10 ` [PATCH 09/21] nfs: don't use nd->intent.open.flags Miklos Szeredi
2012-06-05 13:10 ` [PATCH 10/21] nfs: don't use intents for checking atomic open Miklos Szeredi
2012-06-05 13:10 ` [PATCH 11/21] fuse: implement i_op->atomic_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 12/21] cifs: " Miklos Szeredi
     [not found]   ` <1338901832-14049-13-git-send-email-miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>
2012-07-02 18:54     ` Jeff Layton
2012-07-02 18:54       ` Jeff Layton
2012-06-05 13:10 ` [PATCH 13/21] ceph: remove unused arg from ceph_lookup_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 14/21] ceph: implement i_op->atomic_open() Miklos Szeredi
2012-06-05 13:10 ` [PATCH 15/21] 9p: " Miklos Szeredi
2012-06-05 13:10 ` [PATCH 16/21] vfs: remove open intents from nameidata Miklos Szeredi
2012-06-05 13:10 ` [PATCH 17/21] vfs: do_last(): clean up error handling Miklos Szeredi
2012-06-05 13:10 ` [PATCH 18/21] vfs: do_last(): clean up labels Miklos Szeredi
2012-06-05 13:10 ` [PATCH 19/21] vfs: do_last(): clean up bool Miklos Szeredi
2012-06-05 13:10 ` [PATCH 20/21] vfs: do_last(): clean up retry Miklos Szeredi
2012-06-05 13:10 ` [PATCH 21/21] vfs: move O_DIRECT check to common code Miklos Szeredi
2012-06-05 15:39 ` [PATCH 00/21] vfs: atomic open v6 (part 2) Linus Torvalds
2012-06-05 15:50   ` Miklos Szeredi
2012-06-10  3:49 ` Al Viro
2012-06-10  5:54   ` Al Viro
2012-06-10 11:10   ` Al Viro [this message]
2012-06-10 17:56     ` Al Viro
2012-06-10 22:27       ` Al Viro
2012-06-13 11:21         ` Christoph Hellwig
2012-06-14  8:08           ` Al Viro
2012-06-17 20:37         ` Christoph Hellwig
2012-06-18 11:58           ` Christoph Hellwig
2012-06-18 13:12             ` Christoph Hellwig
2012-06-18 14:27               ` Miklos Szeredi
2012-06-22  8:49                 ` Al Viro
2012-06-22 10:07               ` Al Viro
2012-06-11 10:57       ` Boaz Harrosh
2012-06-11 15:18   ` Miklos Szeredi
2012-06-11 16:33   ` Miklos Szeredi

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=20120610111056.GD30000@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@suse.cz \
    --cc=torvalds@linux-foundation.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.