* [PATCH 0/10] udf: Unify aops
@ 2023-01-24 12:06 Jan Kara
2023-01-24 12:06 ` [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files Jan Kara
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Hello,
this patch series makes UDF use the same address_space_operations for both
normal and in-ICB files as switching aops on live files is prone to races
as spotted by syzbot. When already dealing with this code, switch readpage
function from using kmap_atomic() to use kmap_local_page().
I plan to queue these patches to my tree.
Honza
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:22 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages() Jan Kara
` (8 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make udf_read_folio() handle both normal and in-ICB files.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 15 +++------------
fs/udf/inode.c | 9 ++++++++-
fs/udf/udfdecl.h | 2 ++
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 322115c8369d..2666234a5204 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -38,7 +38,7 @@
#include "udf_i.h"
#include "udf_sb.h"
-static void __udf_adinicb_readpage(struct page *page)
+void udf_adinicb_readpage(struct page *page)
{
struct inode *inode = page->mapping->host;
char *kaddr;
@@ -57,15 +57,6 @@ static void __udf_adinicb_readpage(struct page *page)
kunmap_atomic(kaddr);
}
-static int udf_adinicb_read_folio(struct file *file, struct folio *folio)
-{
- BUG_ON(!folio_test_locked(folio));
- __udf_adinicb_readpage(&folio->page);
- folio_unlock(folio);
-
- return 0;
-}
-
static int udf_adinicb_writepage(struct page *page,
struct writeback_control *wbc)
{
@@ -100,7 +91,7 @@ static int udf_adinicb_write_begin(struct file *file,
*pagep = page;
if (!PageUptodate(page))
- __udf_adinicb_readpage(page);
+ udf_adinicb_readpage(page);
return 0;
}
@@ -127,7 +118,7 @@ static int udf_adinicb_write_end(struct file *file, struct address_space *mappin
const struct address_space_operations udf_adinicb_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
- .read_folio = udf_adinicb_read_folio,
+ .read_folio = udf_read_folio,
.writepage = udf_adinicb_writepage,
.write_begin = udf_adinicb_write_begin,
.write_end = udf_adinicb_write_end,
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 15e0d9f23c06..9ef56574e452 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -191,8 +191,15 @@ static int udf_writepages(struct address_space *mapping,
return mpage_writepages(mapping, wbc, udf_get_block_wb);
}
-static int udf_read_folio(struct file *file, struct folio *folio)
+int udf_read_folio(struct file *file, struct folio *folio)
{
+ struct udf_inode_info *iinfo = UDF_I(file_inode(file));
+
+ if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
+ udf_adinicb_readpage(&folio->page);
+ folio_unlock(folio);
+ return 0;
+ }
return mpage_read_folio(folio, udf_get_block);
}
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 5ba59ab90d48..6b93b393cb46 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -138,6 +138,7 @@ static inline unsigned int udf_dir_entry_len(struct fileIdentDesc *cfi)
/* file.c */
extern long udf_ioctl(struct file *, unsigned int, unsigned long);
+void udf_adinicb_readpage(struct page *page);
/* inode.c */
extern struct inode *__udf_iget(struct super_block *, struct kernel_lb_addr *,
@@ -158,6 +159,7 @@ extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
extern int udf_setsize(struct inode *, loff_t);
extern void udf_evict_inode(struct inode *);
extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
+int udf_read_folio(struct file *file, struct folio *folio);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
2023-01-24 12:06 ` [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:25 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO() Jan Kara
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christoph Hellwig, Jan Kara, syzbot+c27475eb921c46bbdc62,
Christoph Hellwig
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make in-ICB files use udf_writepages().
Reported-by: syzbot+c27475eb921c46bbdc62@syzkaller.appspotmail.com
Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 21 +--------------------
fs/udf/inode.c | 29 ++++++++++++++++++++++++++---
fs/udf/udfdecl.h | 1 +
3 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 2666234a5204..7a8dbad86e41 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -57,25 +57,6 @@ void udf_adinicb_readpage(struct page *page)
kunmap_atomic(kaddr);
}
-static int udf_adinicb_writepage(struct page *page,
- struct writeback_control *wbc)
-{
- struct inode *inode = page->mapping->host;
- char *kaddr;
- struct udf_inode_info *iinfo = UDF_I(inode);
-
- BUG_ON(!PageLocked(page));
-
- kaddr = kmap_atomic(page);
- memcpy(iinfo->i_data + iinfo->i_lenEAttr, kaddr, i_size_read(inode));
- SetPageUptodate(page);
- kunmap_atomic(kaddr);
- mark_inode_dirty(inode);
- unlock_page(page);
-
- return 0;
-}
-
static int udf_adinicb_write_begin(struct file *file,
struct address_space *mapping, loff_t pos,
unsigned len, struct page **pagep,
@@ -119,7 +100,7 @@ const struct address_space_operations udf_adinicb_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = udf_read_folio,
- .writepage = udf_adinicb_writepage,
+ .writepages = udf_writepages,
.write_begin = udf_adinicb_write_begin,
.write_end = udf_adinicb_write_end,
.direct_IO = udf_adinicb_direct_IO,
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 9ef56574e452..54e6127ebf55 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -185,10 +185,33 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
}
}
-static int udf_writepages(struct address_space *mapping,
- struct writeback_control *wbc)
+static int udf_adinicb_writepage(struct page *page,
+ struct writeback_control *wbc, void *data)
{
- return mpage_writepages(mapping, wbc, udf_get_block_wb);
+ struct inode *inode = page->mapping->host;
+ char *kaddr;
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ BUG_ON(!PageLocked(page));
+
+ kaddr = kmap_atomic(page);
+ memcpy(iinfo->i_data + iinfo->i_lenEAttr, kaddr, i_size_read(inode));
+ SetPageUptodate(page);
+ kunmap_atomic(kaddr);
+ unlock_page(page);
+ mark_inode_dirty(inode);
+
+ return 0;
+}
+
+int udf_writepages(struct address_space *mapping, struct writeback_control *wbc)
+{
+ struct inode *inode = mapping->host;
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB)
+ return mpage_writepages(mapping, wbc, udf_get_block_wb);
+ return write_cache_pages(mapping, wbc, udf_adinicb_writepage, NULL);
}
int udf_read_folio(struct file *file, struct folio *folio)
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 6b93b393cb46..48647eab26a6 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -160,6 +160,7 @@ extern int udf_setsize(struct inode *, loff_t);
extern void udf_evict_inode(struct inode *);
extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
int udf_read_folio(struct file *file, struct folio *folio);
+int udf_writepages(struct address_space *mapping, struct writeback_control *wbc);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
2023-01-24 12:06 ` [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files Jan Kara
2023-01-24 12:06 ` [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages() Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:25 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin() Jan Kara
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make in-ICB files use udf_direct_IO().
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 8 +-------
fs/udf/inode.c | 5 ++++-
fs/udf/udfdecl.h | 1 +
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 7a8dbad86e41..6bab6aa7770a 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -76,12 +76,6 @@ static int udf_adinicb_write_begin(struct file *file,
return 0;
}
-static ssize_t udf_adinicb_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
-{
- /* Fallback to buffered I/O. */
- return 0;
-}
-
static int udf_adinicb_write_end(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned copied,
struct page *page, void *fsdata)
@@ -103,7 +97,7 @@ const struct address_space_operations udf_adinicb_aops = {
.writepages = udf_writepages,
.write_begin = udf_adinicb_write_begin,
.write_end = udf_adinicb_write_end,
- .direct_IO = udf_adinicb_direct_IO,
+ .direct_IO = udf_direct_IO,
};
static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 54e6127ebf55..52016c942f68 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -243,7 +243,7 @@ static int udf_write_begin(struct file *file, struct address_space *mapping,
return ret;
}
-static ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
+ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
@@ -251,6 +251,9 @@ static ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
size_t count = iov_iter_count(iter);
ssize_t ret;
+ /* Fallback to buffered IO for in-ICB files */
+ if (UDF_I(inode)->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
+ return 0;
ret = blockdev_direct_IO(iocb, inode, iter, udf_get_block);
if (unlikely(ret < 0 && iov_iter_rw(iter) == WRITE))
udf_write_failed(mapping, iocb->ki_pos + count);
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 48647eab26a6..a851613465c6 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -161,6 +161,7 @@ extern void udf_evict_inode(struct inode *);
extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
int udf_read_folio(struct file *file, struct folio *folio);
int udf_writepages(struct address_space *mapping, struct writeback_control *wbc);
+ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (2 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO() Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:26 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 05/10] udf: Convert all file types to use udf_write_end() Jan Kara
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make in-ICB files use udf_write_begin().
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 21 +--------------------
fs/udf/inode.c | 24 +++++++++++++++++++-----
fs/udf/udfdecl.h | 3 +++
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 6bab6aa7770a..16aecf4b2387 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -57,25 +57,6 @@ void udf_adinicb_readpage(struct page *page)
kunmap_atomic(kaddr);
}
-static int udf_adinicb_write_begin(struct file *file,
- struct address_space *mapping, loff_t pos,
- unsigned len, struct page **pagep,
- void **fsdata)
-{
- struct page *page;
-
- if (WARN_ON_ONCE(pos >= PAGE_SIZE))
- return -EIO;
- page = grab_cache_page_write_begin(mapping, 0);
- if (!page)
- return -ENOMEM;
- *pagep = page;
-
- if (!PageUptodate(page))
- udf_adinicb_readpage(page);
- return 0;
-}
-
static int udf_adinicb_write_end(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned copied,
struct page *page, void *fsdata)
@@ -95,7 +76,7 @@ const struct address_space_operations udf_adinicb_aops = {
.invalidate_folio = block_invalidate_folio,
.read_folio = udf_read_folio,
.writepages = udf_writepages,
- .write_begin = udf_adinicb_write_begin,
+ .write_begin = udf_write_begin,
.write_end = udf_adinicb_write_end,
.direct_IO = udf_direct_IO,
};
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 52016c942f68..7f5b2b1f9847 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -231,16 +231,30 @@ static void udf_readahead(struct readahead_control *rac)
mpage_readahead(rac, udf_get_block);
}
-static int udf_write_begin(struct file *file, struct address_space *mapping,
+int udf_write_begin(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len,
struct page **pagep, void **fsdata)
{
+ struct udf_inode_info *iinfo = UDF_I(file_inode(file));
+ struct page *page;
int ret;
- ret = block_write_begin(mapping, pos, len, pagep, udf_get_block);
- if (unlikely(ret))
- udf_write_failed(mapping, pos + len);
- return ret;
+ if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
+ ret = block_write_begin(mapping, pos, len, pagep,
+ udf_get_block);
+ if (unlikely(ret))
+ udf_write_failed(mapping, pos + len);
+ return ret;
+ }
+ if (WARN_ON_ONCE(pos >= PAGE_SIZE))
+ return -EIO;
+ page = grab_cache_page_write_begin(mapping, 0);
+ if (!page)
+ return -ENOMEM;
+ *pagep = page;
+ if (!PageUptodate(page))
+ udf_adinicb_readpage(page);
+ return 0;
}
ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index a851613465c6..32decf6b6a21 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -161,6 +161,9 @@ extern void udf_evict_inode(struct inode *);
extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
int udf_read_folio(struct file *file, struct folio *folio);
int udf_writepages(struct address_space *mapping, struct writeback_control *wbc);
+int udf_write_begin(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len,
+ struct page **pagep, void **fsdata);
ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 05/10] udf: Convert all file types to use udf_write_end()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (3 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin() Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:27 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap() Jan Kara
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, create udf_write_end() function that is able to handle both
normal and in-ICB files.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 16 +---------------
fs/udf/inode.c | 22 +++++++++++++++++++++-
fs/udf/udfdecl.h | 3 +++
3 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 16aecf4b2387..8a37cd593883 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -57,27 +57,13 @@ void udf_adinicb_readpage(struct page *page)
kunmap_atomic(kaddr);
}
-static int udf_adinicb_write_end(struct file *file, struct address_space *mapping,
- loff_t pos, unsigned len, unsigned copied,
- struct page *page, void *fsdata)
-{
- struct inode *inode = page->mapping->host;
- loff_t last_pos = pos + copied;
- if (last_pos > inode->i_size)
- i_size_write(inode, last_pos);
- set_page_dirty(page);
- unlock_page(page);
- put_page(page);
- return copied;
-}
-
const struct address_space_operations udf_adinicb_aops = {
.dirty_folio = block_dirty_folio,
.invalidate_folio = block_invalidate_folio,
.read_folio = udf_read_folio,
.writepages = udf_writepages,
.write_begin = udf_write_begin,
- .write_end = udf_adinicb_write_end,
+ .write_end = udf_write_end,
.direct_IO = udf_direct_IO,
};
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 7f5b2b1f9847..91758c8d77e5 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -257,6 +257,26 @@ int udf_write_begin(struct file *file, struct address_space *mapping,
return 0;
}
+int udf_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
+{
+ struct inode *inode = file_inode(file);
+ loff_t last_pos;
+
+ if (UDF_I(inode)->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB)
+ return generic_write_end(file, mapping, pos, len, copied, page,
+ fsdata);
+ last_pos = pos + copied;
+ if (last_pos > inode->i_size)
+ i_size_write(inode, last_pos);
+ set_page_dirty(page);
+ unlock_page(page);
+ put_page(page);
+
+ return copied;
+}
+
ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
{
struct file *file = iocb->ki_filp;
@@ -286,7 +306,7 @@ const struct address_space_operations udf_aops = {
.readahead = udf_readahead,
.writepages = udf_writepages,
.write_begin = udf_write_begin,
- .write_end = generic_write_end,
+ .write_end = udf_write_end,
.direct_IO = udf_direct_IO,
.bmap = udf_bmap,
.migrate_folio = buffer_migrate_folio,
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 32decf6b6a21..304c2ec81589 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -164,6 +164,9 @@ int udf_writepages(struct address_space *mapping, struct writeback_control *wbc)
int udf_write_begin(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len,
struct page **pagep, void **fsdata);
+int udf_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata);
ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (4 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 05/10] udf: Convert all file types to use udf_write_end() Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:27 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 07/10] udf: Switch to single address_space_operations Jan Kara
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Add detection of in-ICB files to udf_bmap() and return error in that
case. This will allow us o use single address_space_operations in UDF.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/inode.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 91758c8d77e5..703db2a4516b 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -296,6 +296,10 @@ ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
static sector_t udf_bmap(struct address_space *mapping, sector_t block)
{
+ struct udf_inode_info *iinfo = UDF_I(mapping->host);
+
+ if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
+ return -EINVAL;
return generic_block_bmap(mapping, block, udf_get_block);
}
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 07/10] udf: Switch to single address_space_operations
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (5 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap() Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 08/10] udf: Mark aops implementation static Jan Kara
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Now that udf_aops and udf_adiniicb_aops are functionally identical, just
drop udf_adiniicb_aops.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 10 ----------
fs/udf/inode.c | 8 +-------
fs/udf/namei.c | 10 ++--------
fs/udf/udfdecl.h | 1 -
4 files changed, 3 insertions(+), 26 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 8a37cd593883..84e0b241940d 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -57,16 +57,6 @@ void udf_adinicb_readpage(struct page *page)
kunmap_atomic(kaddr);
}
-const struct address_space_operations udf_adinicb_aops = {
- .dirty_folio = block_dirty_folio,
- .invalidate_folio = block_invalidate_folio,
- .read_folio = udf_read_folio,
- .writepages = udf_writepages,
- .write_begin = udf_write_begin,
- .write_end = udf_write_end,
- .direct_IO = udf_direct_IO,
-};
-
static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 703db2a4516b..5b0be8f281f0 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -364,8 +364,6 @@ int udf_expand_file_adinicb(struct inode *inode)
iinfo->i_alloc_type = ICBTAG_FLAG_AD_SHORT;
else
iinfo->i_alloc_type = ICBTAG_FLAG_AD_LONG;
- /* from now on we have normal address_space methods */
- inode->i_data.a_ops = &udf_aops;
set_page_dirty(page);
unlock_page(page);
up_write(&iinfo->i_data_sem);
@@ -379,7 +377,6 @@ int udf_expand_file_adinicb(struct inode *inode)
kunmap_atomic(kaddr);
unlock_page(page);
iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
- inode->i_data.a_ops = &udf_adinicb_aops;
iinfo->i_lenAlloc = inode->i_size;
up_write(&iinfo->i_data_sem);
}
@@ -1566,10 +1563,7 @@ static int udf_read_inode(struct inode *inode, bool hidden_inode)
case ICBTAG_FILE_TYPE_REGULAR:
case ICBTAG_FILE_TYPE_UNDEF:
case ICBTAG_FILE_TYPE_VAT20:
- if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
- inode->i_data.a_ops = &udf_adinicb_aops;
- else
- inode->i_data.a_ops = &udf_aops;
+ inode->i_data.a_ops = &udf_aops;
inode->i_op = &udf_file_inode_operations;
inode->i_fop = &udf_file_operations;
inode->i_mode |= S_IFREG;
diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 1b0f4c600b63..c043584463d2 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -373,10 +373,7 @@ static int udf_create(struct user_namespace *mnt_userns, struct inode *dir,
if (IS_ERR(inode))
return PTR_ERR(inode);
- if (UDF_I(inode)->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
- inode->i_data.a_ops = &udf_adinicb_aops;
- else
- inode->i_data.a_ops = &udf_aops;
+ inode->i_data.a_ops = &udf_aops;
inode->i_op = &udf_file_inode_operations;
inode->i_fop = &udf_file_operations;
mark_inode_dirty(inode);
@@ -392,10 +389,7 @@ static int udf_tmpfile(struct user_namespace *mnt_userns, struct inode *dir,
if (IS_ERR(inode))
return PTR_ERR(inode);
- if (UDF_I(inode)->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
- inode->i_data.a_ops = &udf_adinicb_aops;
- else
- inode->i_data.a_ops = &udf_aops;
+ inode->i_data.a_ops = &udf_aops;
inode->i_op = &udf_file_inode_operations;
inode->i_fop = &udf_file_operations;
mark_inode_dirty(inode);
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 304c2ec81589..d8c0de3b224e 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -80,7 +80,6 @@ extern const struct inode_operations udf_file_inode_operations;
extern const struct file_operations udf_file_operations;
extern const struct inode_operations udf_symlink_inode_operations;
extern const struct address_space_operations udf_aops;
-extern const struct address_space_operations udf_adinicb_aops;
extern const struct address_space_operations udf_symlink_aops;
struct udf_fileident_iter {
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 08/10] udf: Mark aops implementation static
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (6 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 07/10] udf: Switch to single address_space_operations Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c Jan Kara
2023-01-24 12:06 ` [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page() Jan Kara
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Mark functions implementing aops static since they are not needed
outside of inode.c anymore.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/inode.c | 19 ++++++++++---------
fs/udf/udfdecl.h | 9 ---------
2 files changed, 10 insertions(+), 18 deletions(-)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 5b0be8f281f0..af069a05919c 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -204,7 +204,8 @@ static int udf_adinicb_writepage(struct page *page,
return 0;
}
-int udf_writepages(struct address_space *mapping, struct writeback_control *wbc)
+static int udf_writepages(struct address_space *mapping,
+ struct writeback_control *wbc)
{
struct inode *inode = mapping->host;
struct udf_inode_info *iinfo = UDF_I(inode);
@@ -214,7 +215,7 @@ int udf_writepages(struct address_space *mapping, struct writeback_control *wbc)
return write_cache_pages(mapping, wbc, udf_adinicb_writepage, NULL);
}
-int udf_read_folio(struct file *file, struct folio *folio)
+static int udf_read_folio(struct file *file, struct folio *folio)
{
struct udf_inode_info *iinfo = UDF_I(file_inode(file));
@@ -231,9 +232,9 @@ static void udf_readahead(struct readahead_control *rac)
mpage_readahead(rac, udf_get_block);
}
-int udf_write_begin(struct file *file, struct address_space *mapping,
- loff_t pos, unsigned len,
- struct page **pagep, void **fsdata)
+static int udf_write_begin(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len,
+ struct page **pagep, void **fsdata)
{
struct udf_inode_info *iinfo = UDF_I(file_inode(file));
struct page *page;
@@ -257,9 +258,9 @@ int udf_write_begin(struct file *file, struct address_space *mapping,
return 0;
}
-int udf_write_end(struct file *file, struct address_space *mapping,
- loff_t pos, unsigned len, unsigned copied,
- struct page *page, void *fsdata)
+static int udf_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
{
struct inode *inode = file_inode(file);
loff_t last_pos;
@@ -277,7 +278,7 @@ int udf_write_end(struct file *file, struct address_space *mapping,
return copied;
}
-ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
+static ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
{
struct file *file = iocb->ki_filp;
struct address_space *mapping = file->f_mapping;
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index d8c0de3b224e..337daf97d5b4 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -158,15 +158,6 @@ extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
extern int udf_setsize(struct inode *, loff_t);
extern void udf_evict_inode(struct inode *);
extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
-int udf_read_folio(struct file *file, struct folio *folio);
-int udf_writepages(struct address_space *mapping, struct writeback_control *wbc);
-int udf_write_begin(struct file *file, struct address_space *mapping,
- loff_t pos, unsigned len,
- struct page **pagep, void **fsdata);
-int udf_write_end(struct file *file, struct address_space *mapping,
- loff_t pos, unsigned len, unsigned copied,
- struct page *page, void *fsdata);
-ssize_t udf_direct_IO(struct kiocb *iocb, struct iov_iter *iter);
extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, sector_t *);
int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (7 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 08/10] udf: Mark aops implementation static Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page() Jan Kara
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
udf_adinicb_readpage() is only called from aops functions, move it to
the same file as its callers and also drop the stale comment -
invalidate_lock is protecting us against races with truncate.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/file.c | 19 -------------------
fs/udf/inode.c | 15 +++++++++++++++
fs/udf/udfdecl.h | 1 -
3 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/fs/udf/file.c b/fs/udf/file.c
index 84e0b241940d..26592577b7da 100644
--- a/fs/udf/file.c
+++ b/fs/udf/file.c
@@ -38,25 +38,6 @@
#include "udf_i.h"
#include "udf_sb.h"
-void udf_adinicb_readpage(struct page *page)
-{
- struct inode *inode = page->mapping->host;
- char *kaddr;
- struct udf_inode_info *iinfo = UDF_I(inode);
- loff_t isize = i_size_read(inode);
-
- /*
- * We have to be careful here as truncate can change i_size under us.
- * So just sample it once and use the same value everywhere.
- */
- kaddr = kmap_atomic(page);
- memcpy(kaddr, iinfo->i_data + iinfo->i_lenEAttr, isize);
- memset(kaddr + isize, 0, PAGE_SIZE - isize);
- flush_dcache_page(page);
- SetPageUptodate(page);
- kunmap_atomic(kaddr);
-}
-
static vm_fault_t udf_page_mkwrite(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index af069a05919c..dcd3f1dac227 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -215,6 +215,21 @@ static int udf_writepages(struct address_space *mapping,
return write_cache_pages(mapping, wbc, udf_adinicb_writepage, NULL);
}
+static void udf_adinicb_readpage(struct page *page)
+{
+ struct inode *inode = page->mapping->host;
+ char *kaddr;
+ struct udf_inode_info *iinfo = UDF_I(inode);
+ loff_t isize = i_size_read(inode);
+
+ kaddr = kmap_atomic(page);
+ memcpy(kaddr, iinfo->i_data + iinfo->i_lenEAttr, isize);
+ memset(kaddr + isize, 0, PAGE_SIZE - isize);
+ flush_dcache_page(page);
+ SetPageUptodate(page);
+ kunmap_atomic(kaddr);
+}
+
static int udf_read_folio(struct file *file, struct folio *folio)
{
struct udf_inode_info *iinfo = UDF_I(file_inode(file));
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 337daf97d5b4..88692512a466 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -137,7 +137,6 @@ static inline unsigned int udf_dir_entry_len(struct fileIdentDesc *cfi)
/* file.c */
extern long udf_ioctl(struct file *, unsigned int, unsigned long);
-void udf_adinicb_readpage(struct page *page);
/* inode.c */
extern struct inode *__udf_iget(struct super_block *, struct kernel_lb_addr *,
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page()
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
` (8 preceding siblings ...)
2023-01-24 12:06 ` [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c Jan Kara
@ 2023-01-24 12:06 ` Jan Kara
2023-01-24 13:29 ` Christoph Hellwig
9 siblings, 1 reply; 22+ messages in thread
From: Jan Kara @ 2023-01-24 12:06 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Jan Kara
Instead of using kmap_atomic() use kmap_local_page() in
udf_adinicb_readpage().
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index dcd3f1dac227..5ae29f89869b 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -222,12 +222,12 @@ static void udf_adinicb_readpage(struct page *page)
struct udf_inode_info *iinfo = UDF_I(inode);
loff_t isize = i_size_read(inode);
- kaddr = kmap_atomic(page);
+ kaddr = kmap_local_page(page);
memcpy(kaddr, iinfo->i_data + iinfo->i_lenEAttr, isize);
memset(kaddr + isize, 0, PAGE_SIZE - isize);
flush_dcache_page(page);
SetPageUptodate(page);
- kunmap_atomic(kaddr);
+ kunmap_local(kaddr);
}
static int udf_read_folio(struct file *file, struct folio *folio)
--
2.35.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files
2023-01-24 12:06 ` [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files Jan Kara
@ 2023-01-24 13:22 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:22 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages()
2023-01-24 12:06 ` [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages() Jan Kara
@ 2023-01-24 13:25 ` Christoph Hellwig
2023-01-25 9:28 ` Jan Kara
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:25 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, Christoph Hellwig, syzbot+c27475eb921c46bbdc62,
Christoph Hellwig
As a pure mechanical move this looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
But some comments:
> + struct inode *inode = page->mapping->host;
> + char *kaddr;
> + struct udf_inode_info *iinfo = UDF_I(inode);
> +
> + BUG_ON(!PageLocked(page));
> +
> + kaddr = kmap_atomic(page);
> + memcpy(iinfo->i_data + iinfo->i_lenEAttr, kaddr, i_size_read(inode));
> + SetPageUptodate(page);
> + kunmap_atomic(kaddr);
> + unlock_page(page);
This really should be using memcpy_to_page. And the SetPageUptodate
here in ->writepages loos a little odd as well.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO()
2023-01-24 12:06 ` [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO() Jan Kara
@ 2023-01-24 13:25 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:25 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
On Tue, Jan 24, 2023 at 01:06:14PM +0100, Jan Kara wrote:
> Switching address_space_operations while a file is used is difficult to
> do in a race-free way. To be able to use single address_space_operations
> in UDF, make in-ICB files use udf_direct_IO().
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin()
2023-01-24 12:06 ` [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin() Jan Kara
@ 2023-01-24 13:26 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:26 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 05/10] udf: Convert all file types to use udf_write_end()
2023-01-24 12:06 ` [PATCH 05/10] udf: Convert all file types to use udf_write_end() Jan Kara
@ 2023-01-24 13:27 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:27 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap()
2023-01-24 12:06 ` [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap() Jan Kara
@ 2023-01-24 13:27 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:27 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
On Tue, Jan 24, 2023 at 01:06:17PM +0100, Jan Kara wrote:
> Add detection of in-ICB files to udf_bmap() and return error in that
> case. This will allow us o use single address_space_operations in UDF.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 07/10] udf: Switch to single address_space_operations
2023-01-24 12:06 ` [PATCH 07/10] udf: Switch to single address_space_operations Jan Kara
@ 2023-01-24 13:28 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:28 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 08/10] udf: Mark aops implementation static
2023-01-24 12:06 ` [PATCH 08/10] udf: Mark aops implementation static Jan Kara
@ 2023-01-24 13:28 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:28 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c
2023-01-24 12:06 ` [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c Jan Kara
@ 2023-01-24 13:28 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:28 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page()
2023-01-24 12:06 ` [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page() Jan Kara
@ 2023-01-24 13:29 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-01-24 13:29 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Christoph Hellwig
On Tue, Jan 24, 2023 at 01:06:21PM +0100, Jan Kara wrote:
> Instead of using kmap_atomic() use kmap_local_page() in
> udf_adinicb_readpage().
Looks good. Given how often this pattern is repeated I wonder if
we want a memcpy_to_page_and_pad helper, though.
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages()
2023-01-24 13:25 ` Christoph Hellwig
@ 2023-01-25 9:28 ` Jan Kara
0 siblings, 0 replies; 22+ messages in thread
From: Jan Kara @ 2023-01-25 9:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, linux-fsdevel, syzbot+c27475eb921c46bbdc62, Christoph Hellwig
On Tue 24-01-23 05:25:09, Christoph Hellwig wrote:
> As a pure mechanical move this looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
Thanks!
> But some comments:
>
> > + struct inode *inode = page->mapping->host;
> > + char *kaddr;
> > + struct udf_inode_info *iinfo = UDF_I(inode);
> > +
> > + BUG_ON(!PageLocked(page));
> > +
> > + kaddr = kmap_atomic(page);
> > + memcpy(iinfo->i_data + iinfo->i_lenEAttr, kaddr, i_size_read(inode));
> > + SetPageUptodate(page);
> > + kunmap_atomic(kaddr);
> > + unlock_page(page);
>
> This really should be using memcpy_to_page. And the SetPageUptodate
> here in ->writepages loos a little odd as well.
Good points. Added a cleanup patch for this. And then one more to get rid
of the last remaining kmap_atomic() use in UDF (BTW that would benefit from
memcpy_to_page_page() helper as well).
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-01-25 9:28 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-24 12:06 [PATCH 0/10] udf: Unify aops Jan Kara
2023-01-24 12:06 ` [PATCH 01/10] udf: Unify .read_folio for normal and in-ICB files Jan Kara
2023-01-24 13:22 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 02/10] udf: Convert in-ICB files to use udf_writepages() Jan Kara
2023-01-24 13:25 ` Christoph Hellwig
2023-01-25 9:28 ` Jan Kara
2023-01-24 12:06 ` [PATCH 03/10] udf: Convert in-ICB files to use udf_direct_IO() Jan Kara
2023-01-24 13:25 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 04/10] udf: Convert in-ICB files to use udf_write_begin() Jan Kara
2023-01-24 13:26 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 05/10] udf: Convert all file types to use udf_write_end() Jan Kara
2023-01-24 13:27 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 06/10] udf: Add handling of in-ICB files to udf_bmap() Jan Kara
2023-01-24 13:27 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 07/10] udf: Switch to single address_space_operations Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 08/10] udf: Mark aops implementation static Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 09/10] udf: Move udf_adinicb_readpage() to inode.c Jan Kara
2023-01-24 13:28 ` Christoph Hellwig
2023-01-24 12:06 ` [PATCH 10/10] udf: Switch udf_adinicb_readpage() to kmap_local_page() Jan Kara
2023-01-24 13:29 ` Christoph Hellwig
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.