lkml.org 
[lkml]   [2010]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] ocfs2: No need to zero pages past i_size. i_size v2
    Hi Joel,

    On 07/04/2010 05:33 AM, Joel Becker wrote:
    > Here's the second patch, the one that keeps us from zeroing
    > pages past i_size. This should keep ocfs2 and Dave's writeback patch
    > happy.
    >
    > Joel
    >
    > -------------------------------------------------------
    >
    > When ocfs2 fills a hole, it does so by allocating clusters. When a
    > cluster is larger than the write, ocfs2 must zero the portions of the
    > cluster outside of the write. If the clustersize is smaller than a
    > pagecache page, this is handled by the normal pagecache mechanisms, but
    > when the clustersize is larger than a page, ocfs2's write code will zero
    > the pages adjacent to the write. This makes sure the entire cluster is
    > zeroed correctly.
    >
    > Currently ocfs2 behaves exactly the same when writing past i_size.
    > However, this means ocfs2 is writing zeroed pages for portions of a new
    > cluster that are beyond i_size. The page writeback code isn't expecting
    > this. It treats all pages past the one containing i_size as left behind
    > due to a previous truncate operation.
    >
    > Thankfully, ocfs2 calculates the number of pages it will be working on
    > up front. The rest of the write code merely honors the original
    > calculation. We can simply trim the number of pages to only cover the
    > actual file data.
    >
    > Signed-off-by: Joel Becker<joel.becker@oracle.com>
    > ---
    > fs/ocfs2/aops.c | 15 +++++++++++----
    > 1 files changed, 11 insertions(+), 4 deletions(-)
    >
    > diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
    > index 96e6aeb..e90ad74 100644
    > --- a/fs/ocfs2/aops.c
    > +++ b/fs/ocfs2/aops.c
    <snip>
    > @@ -1142,11 +1143,17 @@ static int ocfs2_grab_pages_for_write(struct address_space *mapping,
    > /*
    > * Figure out how many pages we'll be manipulating here. For
    > * non allocating write, we just change the one
    > - * page. Otherwise, we'll need a whole clusters worth.
    > + * page. Otherwise, we'll need a whole clusters worth. If we're
    > + * writing past i_size, we only need enough pages to cover the
    > + * last page of the write.
    The comments for the whole function before the function name also needs
    this change accordingly?
    > */
    > if (new) {
    > wc->w_num_pages = ocfs2_pages_per_cluster(inode->i_sb);
    > start = ocfs2_align_clusters_to_page_index(inode->i_sb, cpos);
    > + /* This is the index *past* the write */
    > + end_index = ((user_pos + user_len)>> PAGE_CACHE_SHIFT) + 1;
    should it be
    end_index = ((user_pos + user_len - 1) >> PAGE_CACHE_SHIFT) + 1?


    > + if ((start + wc->w_num_pages)> end_index)
    > + wc->w_num_pages = end_index - start;
    I just noticed that the below loop in ocfs2_grab_pages_for_write is
    for (i = 0; i < wc->w_num_pages; i++)

    I guess w_num_pages should be set to end_index -
    start_page_of_the_cluster so that we can make sure we grab all the pages
    in this cluster until i_size?

    Regards,
    Tao


    \
     
     \ /
      Last update: 2010-07-04 17:17    [W:0.026 / U:59.860 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site