On Fri, 28 Oct 2011 09:30:17 -0400
Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> As discussed at KS I've collected up the VFS patches from 3 month worth
> of linux-fsdevel archives.
vfs things..
1: What happened with Andi's "dio: optimize cache misses in the
submission path"? (Against which I have checkpatch fixes and
#include linux/prefetch.h, btw)
2: I'm still sitting on Andy Whitcroft's "readlinkat: ensure we
return ENOENT for the empty pathname for normal lookups" and its
fixup. I'd marked this as needed-in-3.1. Help.
From: Andy Whitcroft <apw@xxxxxxxxxxxxx>
Subject: readlinkat: ensure we return ENOENT for the empty pathname for normal lookups
Since the commit below which added O_PATH support to the *at() calls, the
error return for readlink/readlinkat for the empty pathname has switched
from ENOENT to EINVAL:
commit 65cfc6722361570bfe255698d9cd4dccaf47570d
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Sun Mar 13 15:56:26 2011 -0400
readlinkat(), fchownat() and fstatat() with empty relative pathnames
This is both unexpected for userspace and makes readlink/readlinkat
inconsistant with all other interfaces; and inconsistant with our stated
return for these pathnames.
As the readlinkat call does not have a flags parameter we cannot use the
AT_EMPTY_PATH approach used in the other calls. Therefore expose whether
the original path is infact entry via a new user_path_at_empty() path
lookup function. Use this to determine whether to default to EINVAL or
ENOENT for failures.
Addresses http://bugs.launchpad.net/bugs/817187
[akpm@xxxxxxxxxxxxxxxxxxxx: remove unused getname_flags()]
Signed-off-by: Andy Whitcroft <apw@xxxxxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
fs/namei.c | 19 ++++++++++++++-----
fs/stat.c | 5 +++--
include/linux/namei.h | 1 +
3 files changed, 18 insertions(+), 7 deletions(-)
diff -puN fs/namei.c~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups fs/namei.c
--- a/fs/namei.c~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups
+++ a/fs/namei.c
@@ -137,7 +137,8 @@ static int do_getname(const char __user
return retval;
}
-static char *getname_flags(const char __user * filename, int flags)
+static char *getname_flags_empty(const char __user * filename,
+ int flags, int *empty)
{
char *tmp, *result;
@@ -148,6 +149,8 @@ static char *getname_flags(const char __
result = tmp;
if (retval < 0) {
+ if (retval == -ENOENT && empty)
+ *empty = 1;
if (retval != -ENOENT || !(flags & LOOKUP_EMPTY)) {
__putname(tmp);
result = ERR_PTR(retval);
@@ -160,7 +163,7 @@ static char *getname_flags(const char __
char *getname(const char __user * filename)
{
- return getname_flags(filename, 0);
+ return getname_flags_empty(filename, 0, 0);
}
#ifdef CONFIG_AUDITSYSCALL
@@ -1798,11 +1801,11 @@ struct dentry *lookup_one_len(const char
return __lookup_hash(&this, base, NULL);
}
-int user_path_at(int dfd, const char __user *name, unsigned flags,
- struct path *path)
+int user_path_at_empty(int dfd, const char __user *name, unsigned flags,
+ struct path *path, int *empty)
{
struct nameidata nd;
- char *tmp = getname_flags(name, flags);
+ char *tmp = getname_flags_empty(name, flags, empty);
int err = PTR_ERR(tmp);
if (!IS_ERR(tmp)) {
@@ -1816,6 +1819,12 @@ int user_path_at(int dfd, const char __u
return err;
}
+int user_path_at(int dfd, const char __user *name, unsigned flags,
+ struct path *path)
+{
+ return user_path_at_empty(dfd, name, flags, path, 0);
+}
+
static int user_path_parent(int dfd, const char __user *path,
struct nameidata *nd, char **name)
{
diff -puN fs/stat.c~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups fs/stat.c
--- a/fs/stat.c~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups
+++ a/fs/stat.c
@@ -294,15 +294,16 @@ SYSCALL_DEFINE4(readlinkat, int, dfd, co
{
struct path path;
int error;
+ int empty = 0;
if (bufsiz <= 0)
return -EINVAL;
- error = user_path_at(dfd, pathname, LOOKUP_EMPTY, &path);
+ error = user_path_at_empty(dfd, pathname, LOOKUP_EMPTY, &path, &empty);
if (!error) {
struct inode *inode = path.dentry->d_inode;
- error = -EINVAL;
+ error = (empty) ? -ENOENT : -EINVAL;
if (inode->i_op->readlink) {
error = security_inode_readlink(path.dentry);
if (!error) {
diff -puN include/linux/namei.h~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups include/linux/namei.h
--- a/include/linux/namei.h~readlinkat-ensure-we-return-enoent-for-the-empty-pathname-for-normal-lookups
+++ a/include/linux/namei.h
@@ -67,6 +67,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LA
#define LOOKUP_EMPTY 0x4000
extern int user_path_at(int, const char __user *, unsigned, struct path *);
+extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
#define user_path(name, path) user_path_at(AT_FDCWD, name, LOOKUP_FOLLOW, path)
#define user_lpath(name, path) user_path_at(AT_FDCWD, name, 0, path)
_
3: I'm also sitting on Hans Verkuil's "poll: add
poll_requested_events() function". This is being merged via the v4l
tree, based on looks-ok-to-akpm. AFAIK nobody else has looked at
it. This should be in linux-next via the v4l tree by now, but it
isn't, so something might have gone wrong.
4: I'm still sitting on Steve Rago's "fcntl(F_SETFL): allow setting
of O_SYNC". I have a comment here that you said it "needs an
audit". AFAIK nothing has happened. Should I toss it?
From: Steve Rago <sar@xxxxxxxxxxxx>
Subject: fcntl(F_SETFL): allow setting of O_SYNC
This has probably been a problem since day 1 (I ran into this running the
2.4 kernel years ago; finally got around to fixing it). The problem is
that fcntl(fd, F_SETFL, flags|O_SYNC) appears to work, but silently
ignores the O_SYNC flag. Opening the file with O_SYNC works okay, but
setting it later on via fcntl doesn't work.
akpm: will cause surprise slowdowns of applications such as
http://koders.com/c/fidA34D8D5EE9AA5D0AB0F3C604678E2E935E5B0246.aspx?s=dupa
akpm: maybe we should sync the file when someone sets O_SYNC.
Signed-off-by: Steve Rago <sar@xxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
fs/fcntl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -puN fs/fcntl.c~fcntlf_setfl-allow-setting-of-o_sync fs/fcntl.c
--- a/fs/fcntl.c~fcntlf_setfl-allow-setting-of-o_sync
+++ a/fs/fcntl.c
@@ -143,7 +143,7 @@ SYSCALL_DEFINE1(dup, unsigned int, filde
return ret;
}
-#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
+#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME | O_SYNC)
static int setfl(int fd, struct file * filp, unsigned long arg)
{
_
5: Tao Ma's "fs/direct-io.c: calculate fs_count correctly in
get_more_blocks()" was missed and needs rework as a result of this
merge. Here's what I now have. It compiles.
From: Tao Ma <boyu.mt@xxxxxxxxxx>
Subject: fs/direct-io.c: calculate fs_count correctly in get_more_blocks()
In get_more_blocks(), we use dio_count to calcuate fs_count and do some
tricky things to increase fs_count if dio_count isn't aligned. But
actually it still has some corner cases that can't be coverd. See the
following example:
dio_write foo -s 1024 -w 4096
(direct write 4096 bytes at offset 1024). The same goes if the offset
isn't aligned to fs_blocksize.
In this case, the old calculation counts fs_count to be 1, but actually we
will write into 2 different blocks (if fs_blocksize=4096). The old code
just works, since it will call get_block twice (and may have to allocate
and create extents twice for filesystems like ext4). So we'd better call
get_block just once with the proper fs_count.
Signed-off-by: Tao Ma <boyu.mt@xxxxxxxxxx>
Cc: "Theodore Ts'o" <tytso@xxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
fs/direct-io.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff -puN fs/direct-io.c~fs-direct-ioc-calcuate-fs_count-correctly-in-get_more_blocks fs/direct-io.c
--- a/fs/direct-io.c~fs-direct-ioc-calcuate-fs_count-correctly-in-get_more_blocks
+++ a/fs/direct-io.c
@@ -580,9 +580,8 @@ static int get_more_blocks(struct dio *d
{
int ret;
sector_t fs_startblk; /* Into file, in filesystem-sized blocks */
+ sector_t fs_endblk; /* Into file, in filesystem-sized blocks */
unsigned long fs_count; /* Number of filesystem-sized blocks */
- unsigned long dio_count;/* Number of dio_block-sized blocks */
- unsigned long blkmask;
int create;
/*
@@ -593,11 +592,9 @@ static int get_more_blocks(struct dio *d
if (ret == 0) {
BUG_ON(sdio->block_in_file >= sdio->final_block_in_request);
fs_startblk = sdio->block_in_file >> sdio->blkfactor;
- dio_count = sdio->final_block_in_request - sdio->block_in_file;
- fs_count = dio_count >> sdio->blkfactor;
- blkmask = (1 << sdio->blkfactor) - 1;
- if (dio_count & blkmask)
- fs_count++;
+ fs_endblk = (sdio->final_block_in_request - 1) >>
+ sdio->blkfactor;
+ fs_count = fs_endblk - fs_startblk + 1;
map_bh->b_state = 0;
map_bh->b_size = fs_count << dio->inode->i_blkbits;
_
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
