* [PATCH] fs: Remove set but not checked AOP_FLAG_UNINTERRUPTIBLE flag.
@ 2017-02-17 10:48 Tetsuo Handa
2017-02-17 11:48 ` Jeff Layton
0 siblings, 1 reply; 2+ messages in thread
From: Tetsuo Handa @ 2017-02-17 10:48 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-mm, Tetsuo Handa, Jeff Layton, Nick Piggin
Commit afddba49d18f346e ("fs: introduce write_begin, write_end, and
perform_write aops") introduced AOP_FLAG_UNINTERRUPTIBLE flag which was
checked in pagecache_write_begin(), but that check was removed by
commit 4e02ed4b4a2fae34 ("fs: remove prepare_write/commit_write").
Between these two commits, commit d9414774dc0c7b39 ("cifs: Convert cifs
to new aops.") added a check in cifs_write_begin(), but that check was
soon removed by commit a98ee8c1c707fe32 ("[CIFS] fix regression in
cifs_write_begin/cifs_write_end").
Therefore, AOP_FLAG_UNINTERRUPTIBLE flag is checked nowhere.
Let's remove this flag. This patch has no functionality changes.
Cc: Jeff Layton <jlayton@redhat.com>
Cc: Nick Piggin <npiggin@gmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Documentation/filesystems/vfs.txt | 3 +--
fs/buffer.c | 13 +++++--------
fs/exofs/dir.c | 3 +--
fs/hfs/extent.c | 4 ++--
fs/hfsplus/extents.c | 5 ++---
fs/iomap.c | 13 +++----------
fs/namei.c | 2 +-
include/linux/fs.h | 1 -
mm/filemap.c | 6 ------
9 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index b968084..ff5a09e 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -694,8 +694,7 @@ struct address_space_operations {
write_end: After a successful write_begin, and data copy, write_end must
be called. len is the original len passed to write_begin, and copied
- is the amount that was able to be copied (copied == len is always true
- if write_begin was called with the AOP_FLAG_UNINTERRUPTIBLE flag).
+ is the amount that was able to be copied.
The filesystem must take care of unlocking the page and releasing it
refcount, and updating i_size.
diff --git a/fs/buffer.c b/fs/buffer.c
index 28484b3..3974b89 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2378,8 +2378,7 @@ int generic_cont_expand_simple(struct inode *inode, loff_t size)
goto out;
err = pagecache_write_begin(NULL, mapping, size, 0,
- AOP_FLAG_UNINTERRUPTIBLE|AOP_FLAG_CONT_EXPAND,
- &page, &fsdata);
+ AOP_FLAG_CONT_EXPAND, &page, &fsdata);
if (err)
goto out;
@@ -2414,9 +2413,8 @@ static int cont_expand_zero(struct file *file, struct address_space *mapping,
}
len = PAGE_SIZE - zerofrom;
- err = pagecache_write_begin(file, mapping, curpos, len,
- AOP_FLAG_UNINTERRUPTIBLE,
- &page, &fsdata);
+ err = pagecache_write_begin(file, mapping, curpos, len, 0,
+ &page, &fsdata);
if (err)
goto out;
zero_user(page, zerofrom, len);
@@ -2448,9 +2446,8 @@ static int cont_expand_zero(struct file *file, struct address_space *mapping,
}
len = offset - zerofrom;
- err = pagecache_write_begin(file, mapping, curpos, len,
- AOP_FLAG_UNINTERRUPTIBLE,
- &page, &fsdata);
+ err = pagecache_write_begin(file, mapping, curpos, len, 0,
+ &page, &fsdata);
if (err)
goto out;
zero_user(page, zerofrom, len);
diff --git a/fs/exofs/dir.c b/fs/exofs/dir.c
index 42f9a0a..8eeb694 100644
--- a/fs/exofs/dir.c
+++ b/fs/exofs/dir.c
@@ -405,8 +405,7 @@ int exofs_set_link(struct inode *dir, struct exofs_dir_entry *de,
int err;
lock_page(page);
- err = exofs_write_begin(NULL, page->mapping, pos, len,
- AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
+ err = exofs_write_begin(NULL, page->mapping, pos, len, 0, &page, NULL);
if (err)
EXOFS_ERR("exofs_set_link: exofs_write_begin FAILED => %d\n",
err);
diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index e33a0d3..5d01826 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -485,8 +485,8 @@ void hfs_file_truncate(struct inode *inode)
/* XXX: Can use generic_cont_expand? */
size = inode->i_size - 1;
- res = pagecache_write_begin(NULL, mapping, size+1, 0,
- AOP_FLAG_UNINTERRUPTIBLE, &page, &fsdata);
+ res = pagecache_write_begin(NULL, mapping, size+1, 0, 0,
+ &page, &fsdata);
if (!res) {
res = pagecache_write_end(NULL, mapping, size+1, 0, 0,
page, fsdata);
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index feca524..a3eb640 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -545,9 +545,8 @@ void hfsplus_file_truncate(struct inode *inode)
void *fsdata;
loff_t size = inode->i_size;
- res = pagecache_write_begin(NULL, mapping, size, 0,
- AOP_FLAG_UNINTERRUPTIBLE,
- &page, &fsdata);
+ res = pagecache_write_begin(NULL, mapping, size, 0, 0,
+ &page, &fsdata);
if (res)
return;
res = pagecache_write_end(NULL, mapping, size,
diff --git a/fs/iomap.c b/fs/iomap.c
index 0f85f24..f740ca3 100644
--- a/fs/iomap.c
+++ b/fs/iomap.c
@@ -156,12 +156,6 @@
ssize_t written = 0;
unsigned int flags = AOP_FLAG_NOFS;
- /*
- * Copies from kernel address space cannot fail (NFSD is a big user).
- */
- if (!iter_is_iovec(i))
- flags |= AOP_FLAG_UNINTERRUPTIBLE;
-
do {
struct page *page;
unsigned long offset; /* Offset into pagecache page */
@@ -289,8 +283,7 @@
return PTR_ERR(rpage);
status = iomap_write_begin(inode, pos, bytes,
- AOP_FLAG_NOFS | AOP_FLAG_UNINTERRUPTIBLE,
- &page, iomap);
+ AOP_FLAG_NOFS, &page, iomap);
put_page(rpage);
if (unlikely(status))
return status;
@@ -341,8 +334,8 @@ static int iomap_zero(struct inode *inode, loff_t pos, unsigned offset,
struct page *page;
int status;
- status = iomap_write_begin(inode, pos, bytes,
- AOP_FLAG_UNINTERRUPTIBLE | AOP_FLAG_NOFS, &page, iomap);
+ status = iomap_write_begin(inode, pos, bytes, AOP_FLAG_NOFS, &page,
+ iomap);
if (status)
return status;
diff --git a/fs/namei.c b/fs/namei.c
index e79ac7a..4fd1ee2 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4756,7 +4756,7 @@ int __page_symlink(struct inode *inode, const char *symname, int len, int nofs)
struct page *page;
void *fsdata;
int err;
- unsigned int flags = AOP_FLAG_UNINTERRUPTIBLE;
+ unsigned int flags = 0;
if (nofs)
flags |= AOP_FLAG_NOFS;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index de8ed0b..77a084a 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -250,7 +250,6 @@ enum positive_aop_returns {
AOP_TRUNCATED_PAGE = 0x80001,
};
-#define AOP_FLAG_UNINTERRUPTIBLE 0x0001 /* will not do a short write */
#define AOP_FLAG_CONT_EXPAND 0x0002 /* called from cont_expand */
#define AOP_FLAG_NOFS 0x0004 /* used by filesystem to direct
* helper code (eg buffer layer)
diff --git a/mm/filemap.c b/mm/filemap.c
index 2ba46f4..e16047c 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2790,12 +2790,6 @@ ssize_t generic_perform_write(struct file *file,
ssize_t written = 0;
unsigned int flags = 0;
- /*
- * Copies from kernel address space cannot fail (NFSD is a big user).
- */
- if (!iter_is_iovec(i))
- flags |= AOP_FLAG_UNINTERRUPTIBLE;
-
do {
struct page *page;
unsigned long offset; /* Offset into pagecache page */
--
1.8.3.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] fs: Remove set but not checked AOP_FLAG_UNINTERRUPTIBLE flag.
2017-02-17 10:48 [PATCH] fs: Remove set but not checked AOP_FLAG_UNINTERRUPTIBLE flag Tetsuo Handa
@ 2017-02-17 11:48 ` Jeff Layton
0 siblings, 0 replies; 2+ messages in thread
From: Jeff Layton @ 2017-02-17 11:48 UTC (permalink / raw)
To: Tetsuo Handa, viro; +Cc: linux-fsdevel, linux-mm, Nick Piggin
On Fri, 2017-02-17 at 19:48 +0900, Tetsuo Handa wrote:
> Commit afddba49d18f346e ("fs: introduce write_begin, write_end, and
> perform_write aops") introduced AOP_FLAG_UNINTERRUPTIBLE flag which was
> checked in pagecache_write_begin(), but that check was removed by
> commit 4e02ed4b4a2fae34 ("fs: remove prepare_write/commit_write").
>
> Between these two commits, commit d9414774dc0c7b39 ("cifs: Convert cifs
> to new aops.") added a check in cifs_write_begin(), but that check was
> soon removed by commit a98ee8c1c707fe32 ("[CIFS] fix regression in
> cifs_write_begin/cifs_write_end").
>
> Therefore, AOP_FLAG_UNINTERRUPTIBLE flag is checked nowhere.
> Let's remove this flag. This patch has no functionality changes.
>
> Cc: Jeff Layton <jlayton@redhat.com>
> Cc: Nick Piggin <npiggin@gmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> Documentation/filesystems/vfs.txt | 3 +--
> fs/buffer.c | 13 +++++--------
> fs/exofs/dir.c | 3 +--
> fs/hfs/extent.c | 4 ++--
> fs/hfsplus/extents.c | 5 ++---
> fs/iomap.c | 13 +++----------
> fs/namei.c | 2 +-
> include/linux/fs.h | 1 -
> mm/filemap.c | 6 ------
> 9 files changed, 15 insertions(+), 35 deletions(-)
>
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index b968084..ff5a09e 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -694,8 +694,7 @@ struct address_space_operations {
>
> write_end: After a successful write_begin, and data copy, write_end must
> be called. len is the original len passed to write_begin, and copied
> - is the amount that was able to be copied (copied == len is always true
> - if write_begin was called with the AOP_FLAG_UNINTERRUPTIBLE flag).
> + is the amount that was able to be copied.
>
> The filesystem must take care of unlocking the page and releasing it
> refcount, and updating i_size.
> diff --git a/fs/buffer.c b/fs/buffer.c
> index 28484b3..3974b89 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2378,8 +2378,7 @@ int generic_cont_expand_simple(struct inode *inode, loff_t size)
> goto out;
>
> err = pagecache_write_begin(NULL, mapping, size, 0,
> - AOP_FLAG_UNINTERRUPTIBLE|AOP_FLAG_CONT_EXPAND,
> - &page, &fsdata);
> + AOP_FLAG_CONT_EXPAND, &page, &fsdata);
> if (err)
> goto out;
>
> @@ -2414,9 +2413,8 @@ static int cont_expand_zero(struct file *file, struct address_space *mapping,
> }
> len = PAGE_SIZE - zerofrom;
>
> - err = pagecache_write_begin(file, mapping, curpos, len,
> - AOP_FLAG_UNINTERRUPTIBLE,
> - &page, &fsdata);
> + err = pagecache_write_begin(file, mapping, curpos, len, 0,
> + &page, &fsdata);
> if (err)
> goto out;
> zero_user(page, zerofrom, len);
> @@ -2448,9 +2446,8 @@ static int cont_expand_zero(struct file *file, struct address_space *mapping,
> }
> len = offset - zerofrom;
>
> - err = pagecache_write_begin(file, mapping, curpos, len,
> - AOP_FLAG_UNINTERRUPTIBLE,
> - &page, &fsdata);
> + err = pagecache_write_begin(file, mapping, curpos, len, 0,
> + &page, &fsdata);
> if (err)
> goto out;
> zero_user(page, zerofrom, len);
> diff --git a/fs/exofs/dir.c b/fs/exofs/dir.c
> index 42f9a0a..8eeb694 100644
> --- a/fs/exofs/dir.c
> +++ b/fs/exofs/dir.c
> @@ -405,8 +405,7 @@ int exofs_set_link(struct inode *dir, struct exofs_dir_entry *de,
> int err;
>
> lock_page(page);
> - err = exofs_write_begin(NULL, page->mapping, pos, len,
> - AOP_FLAG_UNINTERRUPTIBLE, &page, NULL);
> + err = exofs_write_begin(NULL, page->mapping, pos, len, 0, &page, NULL);
> if (err)
> EXOFS_ERR("exofs_set_link: exofs_write_begin FAILED => %d\n",
> err);
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index e33a0d3..5d01826 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -485,8 +485,8 @@ void hfs_file_truncate(struct inode *inode)
>
> /* XXX: Can use generic_cont_expand? */
> size = inode->i_size - 1;
> - res = pagecache_write_begin(NULL, mapping, size+1, 0,
> - AOP_FLAG_UNINTERRUPTIBLE, &page, &fsdata);
> + res = pagecache_write_begin(NULL, mapping, size+1, 0, 0,
> + &page, &fsdata);
> if (!res) {
> res = pagecache_write_end(NULL, mapping, size+1, 0, 0,
> page, fsdata);
> diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> index feca524..a3eb640 100644
> --- a/fs/hfsplus/extents.c
> +++ b/fs/hfsplus/extents.c
> @@ -545,9 +545,8 @@ void hfsplus_file_truncate(struct inode *inode)
> void *fsdata;
> loff_t size = inode->i_size;
>
> - res = pagecache_write_begin(NULL, mapping, size, 0,
> - AOP_FLAG_UNINTERRUPTIBLE,
> - &page, &fsdata);
> + res = pagecache_write_begin(NULL, mapping, size, 0, 0,
> + &page, &fsdata);
> if (res)
> return;
> res = pagecache_write_end(NULL, mapping, size,
> diff --git a/fs/iomap.c b/fs/iomap.c
> index 0f85f24..f740ca3 100644
> --- a/fs/iomap.c
> +++ b/fs/iomap.c
> @@ -156,12 +156,6 @@
> ssize_t written = 0;
> unsigned int flags = AOP_FLAG_NOFS;
>
> - /*
> - * Copies from kernel address space cannot fail (NFSD is a big user).
> - */
> - if (!iter_is_iovec(i))
> - flags |= AOP_FLAG_UNINTERRUPTIBLE;
> -
> do {
> struct page *page;
> unsigned long offset; /* Offset into pagecache page */
> @@ -289,8 +283,7 @@
> return PTR_ERR(rpage);
>
> status = iomap_write_begin(inode, pos, bytes,
> - AOP_FLAG_NOFS | AOP_FLAG_UNINTERRUPTIBLE,
> - &page, iomap);
> + AOP_FLAG_NOFS, &page, iomap);
> put_page(rpage);
> if (unlikely(status))
> return status;
> @@ -341,8 +334,8 @@ static int iomap_zero(struct inode *inode, loff_t pos, unsigned offset,
> struct page *page;
> int status;
>
> - status = iomap_write_begin(inode, pos, bytes,
> - AOP_FLAG_UNINTERRUPTIBLE | AOP_FLAG_NOFS, &page, iomap);
> + status = iomap_write_begin(inode, pos, bytes, AOP_FLAG_NOFS, &page,
> + iomap);
> if (status)
> return status;
>
> diff --git a/fs/namei.c b/fs/namei.c
> index e79ac7a..4fd1ee2 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4756,7 +4756,7 @@ int __page_symlink(struct inode *inode, const char *symname, int len, int nofs)
> struct page *page;
> void *fsdata;
> int err;
> - unsigned int flags = AOP_FLAG_UNINTERRUPTIBLE;
> + unsigned int flags = 0;
> if (nofs)
> flags |= AOP_FLAG_NOFS;
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index de8ed0b..77a084a 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -250,7 +250,6 @@ enum positive_aop_returns {
> AOP_TRUNCATED_PAGE = 0x80001,
> };
>
> -#define AOP_FLAG_UNINTERRUPTIBLE 0x0001 /* will not do a short write */
> #define AOP_FLAG_CONT_EXPAND 0x0002 /* called from cont_expand */
> #define AOP_FLAG_NOFS 0x0004 /* used by filesystem to direct
> * helper code (eg buffer layer)
Nit: should we shift the flag range down here, since we're removing 0x1?
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 2ba46f4..e16047c 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2790,12 +2790,6 @@ ssize_t generic_perform_write(struct file *file,
> ssize_t written = 0;
> unsigned int flags = 0;
>
> - /*
> - * Copies from kernel address space cannot fail (NFSD is a big user).
> - */
> - if (!iter_is_iovec(i))
> - flags |= AOP_FLAG_UNINTERRUPTIBLE;
> -
> do {
> struct page *page;
> unsigned long offset; /* Offset into pagecache page */
I always like removing cruft like this.
Reviewed-by: Jeff Layton <jlayton@redhat.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-02-17 11:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-17 10:48 [PATCH] fs: Remove set but not checked AOP_FLAG_UNINTERRUPTIBLE flag Tetsuo Handa
2017-02-17 11:48 ` Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).