lkml.org 
[lkml]   [2012]   [Jul]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/7] Use vfs __set_page_dirty interface instead of doing it inside filesystem
On 06/29/2012 01:21 PM, Sage Weil wrote:
> On Thu, 28 Jun 2012, Sha Zhengju wrote:
>
>> From: Sha Zhengju<handai.szj@taobao.com>
>>
>> Following we will treat SetPageDirty and dirty page accounting as an integrated
>> operation. Filesystems had better use vfs interface directly to avoid those details.
>>
>> Signed-off-by: Sha Zhengju<handai.szj@taobao.com>
>> ---
>> fs/buffer.c | 2 +-
>> fs/ceph/addr.c | 20 ++------------------
>> include/linux/buffer_head.h | 2 ++
>> 3 files changed, 5 insertions(+), 19 deletions(-)
>>
>> diff --git a/fs/buffer.c b/fs/buffer.c
>> index e8d96b8..55522dd 100644
>> --- a/fs/buffer.c
>> +++ b/fs/buffer.c
>> @@ -610,7 +610,7 @@ EXPORT_SYMBOL(mark_buffer_dirty_inode);
>> * If warn is true, then emit a warning if the page is not uptodate and has
>> * not been truncated.
>> */
>> -static int __set_page_dirty(struct page *page,
>> +int __set_page_dirty(struct page *page,
>> struct address_space *mapping, int warn)
>> {
>> if (unlikely(!mapping))
> This also needs an EXPORT_SYMBOL(__set_page_dirty) to allow ceph to
> continue to build as a module.
>
> With that fixed, the ceph bits are a welcome cleanup!
>
> Acked-by: Sage Weil<sage@inktank.com>

Further, I check the path again and may it be reworked as follows to
avoid undo?

__set_page_dirty();
__set_page_dirty();
ceph operations; ==> if (page->mapping)
if (page->mapping) ceph
operations;
;
else
undo = 1;
if (undo)
xxx;



Thanks,
Sha

>> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
>> index 8b67304..d028fbe 100644
>> --- a/fs/ceph/addr.c
>> +++ b/fs/ceph/addr.c
>> @@ -5,6 +5,7 @@
>> #include<linux/mm.h>
>> #include<linux/pagemap.h>
>> #include<linux/writeback.h> /* generic_writepages */
>> +#include<linux/buffer_head.h>
>> #include<linux/slab.h>
>> #include<linux/pagevec.h>
>> #include<linux/task_io_accounting_ops.h>
>> @@ -73,14 +74,8 @@ static int ceph_set_page_dirty(struct page *page)
>> int undo = 0;
>> struct ceph_snap_context *snapc;
>>
>> - if (unlikely(!mapping))
>> - return !TestSetPageDirty(page);
>> -
>> - if (TestSetPageDirty(page)) {
>> - dout("%p set_page_dirty %p idx %lu -- already dirty\n",
>> - mapping->host, page, page->index);
>> + if (!__set_page_dirty(page, mapping, 1))
>> return 0;
>> - }
>>
>> inode = mapping->host;
>> ci = ceph_inode(inode);
>> @@ -107,14 +102,7 @@ static int ceph_set_page_dirty(struct page *page)
>> snapc, snapc->seq, snapc->num_snaps);
>> spin_unlock(&ci->i_ceph_lock);
>>
>> - /* now adjust page */
>> - spin_lock_irq(&mapping->tree_lock);
>> if (page->mapping) { /* Race with truncate? */
>> - WARN_ON_ONCE(!PageUptodate(page));
>> - account_page_dirtied(page, page->mapping);
>> - radix_tree_tag_set(&mapping->page_tree,
>> - page_index(page), PAGECACHE_TAG_DIRTY);
>> -
>> /*
>> * Reference snap context in page->private. Also set
>> * PagePrivate so that we get invalidatepage callback.
>> @@ -126,14 +114,10 @@ static int ceph_set_page_dirty(struct page *page)
>> undo = 1;
>> }
>>
>> - spin_unlock_irq(&mapping->tree_lock);
>> -
>> if (undo)
>> /* whoops, we failed to dirty the page */
>> ceph_put_wrbuffer_cap_refs(ci, 1, snapc);
>>
>> - __mark_inode_dirty(mapping->host, I_DIRTY_PAGES);
>> -
>> BUG_ON(!PageDirty(page));
>> return 1;
>> }
>> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
>> index 458f497..0a331a8 100644
>> --- a/include/linux/buffer_head.h
>> +++ b/include/linux/buffer_head.h
>> @@ -336,6 +336,8 @@ static inline void lock_buffer(struct buffer_head *bh)
>> }
>>
>> extern int __set_page_dirty_buffers(struct page *page);
>> +extern int __set_page_dirty(struct page *page,
>> + struct address_space *mapping, int warn);
>>
>> #else /* CONFIG_BLOCK */
>>
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>



\
 
 \ /
  Last update: 2012-07-02 11:02    [W:0.402 / U:0.800 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site