All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: linux-kernel@vger.kernel.org, Miklos Szeredi <mszeredi@suse.cz>
Subject: Re: [Bisected] commit 71574865 (vfs: do_last(): common slow lookup) breaks CUPS printing
Date: Mon, 30 Jul 2012 08:56:11 +0100	[thread overview]
Message-ID: <20120730075611.GD6481@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20120730071023.GA273@x4>

On Mon, Jul 30, 2012 at 09:10:23AM +0200, Markus Trippelsdorf wrote:
> 
> Looks like you're right. The first warning happens during startup. The last one
> when I print a test page (which now succeeds).  
> Thanks Al.

> WTF: open("/dev/input/mice", 34946)
> WTF: open("/dev/usblp0", 32898)
> WTF: open("/dev/usb/lp0", 32898)

Ahhh...  OK, yes - it's the case we had missed (and where the manpage
needs correction, BTW).  O_EXCL for *devices* has additional semantics;
it's not "fail if exists", it's "fail if already opened by somebody".
No need to pester CUPS folks (except that I really hope that this
open of /dev/input/mice does *not* come from them)...

All right, the proper fix is *not* removing O_EXCL from flags; we want
it to reach ->f_flags, so that device open would work correctly.  I think
we need this, but I wonder if that's all; ->atomic_open() instances
might need to be corrected as well.  I've pushed this into for-next/for-linus
for now:

commit f8310c59201b183ebee2e3fe0c7242f5729be0af
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Mon Jul 30 11:50:30 2012 +0400

    fix O_EXCL handling for devices
    
    O_EXCL without O_CREAT has different semantics; it's "fail if already opened",
    not "fail if already exists".  commit 71574865 broke that...
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/namei.c b/fs/namei.c
index 618d353..e133bf3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2418,7 +2418,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 	if ((open_flag & O_CREAT) && !IS_POSIXACL(dir))
 		mode &= ~current_umask();
 
-	if (open_flag & O_EXCL) {
+	if ((open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT)) {
 		open_flag &= ~O_TRUNC;
 		*opened |= FILE_CREATED;
 	}
@@ -2742,7 +2742,7 @@ retry_lookup:
 	}
 
 	error = -EEXIST;
-	if (open_flag & O_EXCL)
+	if ((open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT))
 		goto exit_dput;
 
 	error = follow_managed(path, nd->flags);


  reply	other threads:[~2012-07-30  7:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-28 10:43 [Bisected] commit 71574865 (vfs: do_last(): common slow lookup) breaks CUPS printing Markus Trippelsdorf
2012-07-30  6:50 ` Al Viro
2012-07-30  7:10   ` Markus Trippelsdorf
2012-07-30  7:56     ` Al Viro [this message]
2012-07-30  8:05       ` Markus Trippelsdorf
2012-07-30  8:26         ` Al Viro

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=20120730075611.GD6481@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=markus@trippelsdorf.de \
    --cc=mszeredi@suse.cz \
    /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.