libfs: Open code simple_commit_write into only user
PATCH 1/1 libfs: Open code simple_commit_write into only user
* simple_commit_write was only called by simple_write_end. Open coding it makes it tiny bit less heavy on the arithmetic and much more readable.
* While at it use zero_user() for clearing a partial page.
* While at it add a docbook comment for simple_write_end.
Signed-off-by Boaz Harrosh <[email protected]>
--- fs/libfs.c | 59 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 35 insertions(+), 24 deletions(-)
fs/libfs.c +35 -24
@@ -370,40 +370,51 @@ int simple_write_begin(struct file *file, struct address_space *mapping,return simple_prepare_write (file, page, from, from+len); } -static int simple_commit_write (struct file *file,struct page *page, -unsigned from,unsigned to) -{ -struct inode *inode = page->mapping->host; -loff_t pos = ((loff_t )page->index <<PAGE_CACHE_SHIFT ) + to; - -if (!PageUptodate (page)) -SetPageUptodate (page); -/* -* No need to use i_size_read() here, the i_size -* cannot change under us because we hold the i_mutex. -*/ -if (pos > inode->i_size) -i_size_write (inode, pos); -set_page_dirty (page); -return 0 ; -} - +/** +* simple_write_end - .write_end helper for none block-device FSs +* @available: See .write_end of address_space_operations +* @file: " +* @mapping: " +* @pos: " +* @len: " +* @copied: " +* @page: " +* @fsdata: " +* +* simple_write_end does the minimum needed for updating a page after writing is +* done. It has the same API signature as the .write_end of +* address_space_operations vector. So it can just be set onto .write_end for +* FSs that don't need any other processing. i_mutex is assumed to be held. +* Block based filesystems should use generic_write_end(). +* NOTE: Even though i_size might get updated by this function mark_inode_dirty +* is not called, so a filesystem that actually does store data in .write_inode +* should extend on what's done here with a call to mark_inode_dirty() in the +* case that i_size has changed. +**/ int simple_write_end (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned copied,struct page *page,void *fsdata) { -unsigned from = pos & (PAGE_CACHE_SIZE -1 ); +struct inode *inode = page->mapping->host; +loff_t last_pos = pos + copied;/* zero the stale part of the page if we did a short copy */ if (copied < len) { -void *kaddr =kmap_atomic (page,KM_USER0 ); -memset (kaddr + from + copied,0 , len - copied); -flush_dcache_page (page); -kunmap_atomic (kaddr,KM_USER0 ); +unsigned from = pos & (PAGE_CACHE_SIZE -1 ); + +zero_user (page, from + copied, len - copied); } -simple_commit_write (file, page, from, from+copied); +if (!PageUptodate (page)) +SetPageUptodate (page); +/* +* No need to use i_size_read() here, the i_size +* cannot change under us because we hold the i_mutex. +*/ +if (last_pos > inode->i_size) +i_size_write (inode, last_pos); +set_page_dirty (page);unlock_page (page);page_cache_release (page);
Signature · 1 line
1.6.5.2
Quoted 9 lines · Boaz Harrosh
*
simple_commit_writewas only called bysimple_write_end. Open coding it makes it tiny bit less heavy on the arithmetic and much more readable.* While at it use
zero_user()for clearing a partial page.
* While at it add a docbook comment forsimple_write_end.Signed-off-by Boaz Harrosh <[email protected]>
Al Hi
When looking in fs/libfs.c I can see that also simple_prepare_write() is only used once, locally, but is exported nonetheless. Has its time expired by now?
[And what does EXPORT_UNUSED_SYMBOL() means ?].
This patch was triggered by this fix:
http://marc.info/?l=linux-fsdevel&m=126193226815716&w=2
Thanks
Boaz
Quoted 84 lines
--- fs/libfs.c | 59 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 35 insertions(+), 24 deletions(-)fs/libfs.c +35
-24@@ -370,40 +370,51 @@ int simple_write_begin(struct file *file, struct address_space *mapping,return simple_prepare_write (file, page, from, from+len); } -static int simple_commit_write (struct file *file,struct page *page, -unsigned from,unsigned to) -{ -struct inode *inode = page->mapping->host; -loff_t pos = ((loff_t )page->index <<PAGE_CACHE_SHIFT ) + to; - -if (!PageUptodate (page)) -SetPageUptodate (page); -/* -* No need to use i_size_read() here, the i_size -* cannot change under us because we hold the i_mutex. -*/ -if (pos > inode->i_size) -i_size_write (inode, pos); -set_page_dirty (page); -return 0 ; -} - +/** +* simple_write_end - .write_end helper for none block-device FSs +* @available: See .write_end of address_space_operations +* @file: " +* @mapping: " +* @pos: " +* @len: " +* @copied: " +* @page: " +* @fsdata: " +* +* simple_write_end does the minimum needed for updating a page after writing is +* done. It has the same API signature as the .write_end of +* address_space_operations vector. So it can just be set onto .write_end for +* FSs that don't need any other processing. i_mutex is assumed to be held. +* Block based filesystems should use generic_write_end(). +* NOTE: Even though i_size might get updated by this function mark_inode_dirty +* is not called, so a filesystem that actually does store data in .write_inode +* should extend on what's done here with a call to mark_inode_dirty() in the +* case that i_size has changed. +**/ int simple_write_end (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned copied,struct page *page,void *fsdata) { -unsigned from = pos & (PAGE_CACHE_SIZE -1 ); +struct inode *inode = page->mapping->host; +loff_t last_pos = pos + copied;/* zero the stale part of the page if we did a short copy */ if (copied < len) { -void *kaddr =kmap_atomic (page,KM_USER0 ); -memset (kaddr + from + copied,0 , len - copied); -flush_dcache_page (page); -kunmap_atomic (kaddr,KM_USER0 ); +unsigned from = pos & (PAGE_CACHE_SIZE -1 ); + +zero_user (page, from + copied, len - copied); } -simple_commit_write (file, page, from, from+copied); +if (!PageUptodate (page)) +SetPageUptodate (page); +/* +* No need to use i_size_read() here, the i_size +* cannot change under us because we hold the i_mutex. +*/ +if (last_pos > inode->i_size) +i_size_write (inode, last_pos); +set_page_dirty (page);unlock_page (page);page_cache_release (page);
Quoted 7 lines · On Wed, 30 Dec 2009 17:40:35 +0200 Boaz Harrosh wrote:
*
simple_commit_writewas only called bysimple_write_end. Open coding it makes it tiny bit less heavy on the arithmetic and much more readable.* While at it use
zero_user()for clearing a partial page.
* While at it add a docbook comment forsimple_write_end.
Yep, thanks.
Quoted 23 lines
Signed-off-by Boaz Harrosh <[email protected]>--- fs/libfs.c | 59 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 35 insertions(+), 24 deletions(-)fs/libfs.c +0
-10@@ -370,40 +370,51 @@ int simple_write_begin(struct file *file, struct address_space *mapping,return simple_prepare_write (file, page, from, from+len); } -static int simple_commit_write (struct file *file,struct page *page, -unsigned from,unsigned to) -{ -struct inode *inode = page->mapping->host; -loff_t pos = ((loff_t )page->index <<PAGE_CACHE_SHIFT ) + to; - -if (!PageUptodate (page)) -SetPageUptodate (page); -/* -* No need to use i_size_read() here, the i_size
fs/libfs.c int simple_write_begin(struct file *file, struct address_space *mapping, · line 374
Quoted 10 lines
fs/libfs.c +2
-8-* cannot change under us because we hold the i_mutex. -*/ -if (pos > inode->i_size) -i_size_write (inode, pos); -set_page_dirty (page); -return 0 ; -} - +/** +* simple_write_end - .write_end helper for none block-device FSs
for non-block-device FSes
(well, I would spell it "FSes")
fs/libfs.c int simple_write_begin(struct file *file, struct address_space *mapping, · line 387
Quoted 13 lines
fs/libfs.c +13
-0+* @available: See .write_end of address_space_operations +* @file: " +* @mapping: " +* @pos: " +* @len: " +* @copied: " +* @page: " +* @fsdata: " +* +* simple_write_end does the minimum needed for updating a page after writing is +* done. It has the same API signature as the .write_end of +* address_space_operations vector. So it can just be set onto .write_end for +* FSs that don't need any other processing. i_mutex is assumed to be held.
FSes
fs/libfs.c int simple_write_begin(struct file *file, struct address_space *mapping, · line 389
Quoted 2 lines
fs/libfs.c +2
-0+* Block based filesystems should use generic_write_end(). +* NOTE: Even though i_size might get updated by this function mark_inode_dirty
function,
fs/libfs.c int simple_write_begin(struct file *file, struct address_space *mapping, · line 393
Quoted 4 lines
fs/libfs.c +4
-0+* is not called, so a filesystem that actually does store data in .write_inode +* should extend on what's done here with a call to mark_inode_dirty() in the +* case that i_size has changed. +**/
*/
Quoted 35 lines
fs/libfs.c +14
-6int simple_write_end (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned copied,struct page *page,void *fsdata) { -unsigned from = pos & (PAGE_CACHE_SIZE -1 ); +struct inode *inode = page->mapping->host; +loff_t last_pos = pos + copied;/* zero the stale part of the page if we did a short copy */ if (copied < len) { -void *kaddr =kmap_atomic (page,KM_USER0 ); -memset (kaddr + from + copied,0 , len - copied); -flush_dcache_page (page); -kunmap_atomic (kaddr,KM_USER0 ); +unsigned from = pos & (PAGE_CACHE_SIZE -1 ); + +zero_user (page, from + copied, len - copied); } -simple_commit_write (file, page, from, from+copied); +if (!PageUptodate (page)) +SetPageUptodate (page); +/* +* No need to use i_size_read() here, the i_size +* cannot change under us because we hold the i_mutex. +*/ +if (last_pos > inode->i_size) +i_size_write (inode, last_pos); +set_page_dirty (page);unlock_page (page);page_cache_release (page);Signature · 1 line
1.6.5.2
---
~Randy
PATCH 1/1 libfs: Open code simple_commit_write into only user
In reply to Boaz Harrosh* simple_commit_write was only called by simple_write_end. Open coding it makes it tiny bit less heavy on the arithmetic and much more readable.
* While at it use zero_user() for clearing a partial page.
* While at it add a docbook comment for simple_write_end.
Signed-off-by Boaz Harrosh <[email protected]>
--- fs/libfs.c | 59 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 35 insertions(+), 24 deletions(-)
fs/libfs.c +35 -24
@@ -370,40 +370,51 @@ int simple_write_begin(struct file *file, struct address_space *mapping,return simple_prepare_write (file, page, from, from+len); } -static int simple_commit_write (struct file *file,struct page *page, -unsigned from,unsigned to) -{ -struct inode *inode = page->mapping->host; -loff_t pos = ((loff_t )page->index <<PAGE_CACHE_SHIFT ) + to; - -if (!PageUptodate (page)) -SetPageUptodate (page); -/* -* No need to use i_size_read() here, the i_size -* cannot change under us because we hold the i_mutex. -*/ -if (pos > inode->i_size) -i_size_write (inode, pos); -set_page_dirty (page); -return 0 ; -} - +/** +* simple_write_end - .write_end helper for non-block-device FSes +* @available: See .write_end of address_space_operations +* @file: " +* @mapping: " +* @pos: " +* @len: " +* @copied: " +* @page: " +* @fsdata: " +* +* simple_write_end does the minimum needed for updating a page after writing is +* done. It has the same API signature as the .write_end of +* address_space_operations vector. So it can just be set onto .write_end for +* FSes that don't need any other processing. i_mutex is assumed to be held. +* Block based filesystems should use generic_write_end(). +* NOTE: Even though i_size might get updated by this function, mark_inode_dirty +* is not called, so a filesystem that actually does store data in .write_inode +* should extend on what's done here with a call to mark_inode_dirty() in the +* case that i_size has changed. +*/ int simple_write_end (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned copied,struct page *page,void *fsdata) { -unsigned from = pos & (PAGE_CACHE_SIZE -1 ); +struct inode *inode = page->mapping->host; +loff_t last_pos = pos + copied;/* zero the stale part of the page if we did a short copy */ if (copied < len) { -void *kaddr =kmap_atomic (page,KM_USER0 ); -memset (kaddr + from + copied,0 , len - copied); -flush_dcache_page (page); -kunmap_atomic (kaddr,KM_USER0 ); +unsigned from = pos & (PAGE_CACHE_SIZE -1 ); + +zero_user (page, from + copied, len - copied); } -simple_commit_write (file, page, from, from+copied); +if (!PageUptodate (page)) +SetPageUptodate (page); +/* +* No need to use i_size_read() here, the i_size +* cannot change under us because we hold the i_mutex. +*/ +if (last_pos > inode->i_size) +i_size_write (inode, last_pos); +set_page_dirty (page);unlock_page (page);page_cache_release (page);
Signature · 1 line
1.6.6
Quoted 10 lines · Boaz Harrosh
*
simple_commit_writewas only called bysimple_write_end. Open coding it makes it tiny bit less heavy on the arithmetic and much more readable.* While at it use
zero_user()for clearing a partial page.
* While at it add a docbook comment forsimple_write_end.Signed-off-by Boaz Harrosh <[email protected]>---
Al should I do the same for simple_prepare_write()?
It is EXPORT_UNUSED_SYMBOL() now, could I remove that export?
Boaz
Quoted 4 lines · Boaz Harrosh
Al should I do the same for
simple_prepare_write()?It is
EXPORT_UNUSED_SYMBOL()now, could I remove that export?
Go ahead and kill it.
PATCH 2/2 libfs: Unexport and kill simple_prepare_write
In reply to Boaz HarroshRemove the EXPORT_UNUSED_SYMBOL of simple_prepare_write
Collapse simple_prepare_write into it's only caller, though making it simpler and clearer to understand.
Signed-off-by Boaz Harrosh <[email protected]>
--- fs/libfs.c | 22 ++++++---------------- include/linux/fs.h | 2 -- 2 files changed, 6 insertions(+), 18 deletions(-)
fs/libfs.c +6 -16
@@ -338,28 +338,14 @@ int simple_readpage(struct file *file, struct page *page)return 0 ; } -int simple_prepare_write (struct file *file,struct page *page, -unsigned from,unsigned to) -{ -if (!PageUptodate (page)) { -if (to - from !=PAGE_CACHE_SIZE ) -zero_user_segments (page, -0 , from, - to,PAGE_CACHE_SIZE ); - } -return 0 ; -} -int simple_write_begin (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned flags,struct page **pagep,void **fsdata) {struct page *page;pgoff_t index; -unsigned from; index = pos >>PAGE_CACHE_SHIFT ; - from = pos & (PAGE_CACHE_SIZE -1 ); page =grab_cache_page_write_begin (mapping, index, flags);if (!page)
@@ -367,7 +353,12 @@ int simple_write_begin(struct file *file, struct address_space *mapping, *pagep = page; -return simple_prepare_write (file, page, from, from+len); +if (!PageUptodate (page) && (len !=PAGE_CACHE_SIZE )) { +unsigned from = pos & (PAGE_CACHE_SIZE -1 ); + +zero_user_segments (page,0 , from, from + len,PAGE_CACHE_SIZE ); + } +return 0 ; }/**
@@ -864,7 +855,6 @@ EXPORT_SYMBOL(simple_getattr);EXPORT_SYMBOL (simple_link);EXPORT_SYMBOL (simple_lookup);EXPORT_SYMBOL (simple_pin_fs); -EXPORT_UNUSED_SYMBOL (simple_prepare_write);EXPORT_SYMBOL (simple_readpage);EXPORT_SYMBOL (simple_release_fs);EXPORT_SYMBOL (simple_rename);
include/linux/fs.h +0 -2
@@ -2340,8 +2340,6 @@ extern int simple_rename(struct inode *, struct dentry *, struct inode *, structextern int simple_sync_file (struct file *,struct dentry *,int );extern int simple_empty (struct dentry *);extern int simple_readpage (struct file *file,struct page *page); -extern int simple_prepare_write (struct file *file,struct page *page, -unsigned offset,unsigned to);extern int simple_write_begin (struct file *file,struct address_space *mapping,loff_t pos,unsigned len,unsigned flags,struct page **pagep,void **fsdata);
Signature · 1 line
1.6.6