lkml.org 
[lkml]   [2005]   [Feb]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: RFC: [PATCH-2.6] Add helper function to lock multiple page cache pages.
    On Wed, Feb 02, 2005 at 03:12:50PM +0000, Anton Altaparmakov wrote:

    I think the below loop would be clearer as a for loop ...

    err = 0;
    for (nr = 0; nr < nr_pages; nr++, start++) {
    if (start == lp_idx) {
    pages[nr] = locked_page;
    if (!nr)
    continue;
    lock_page(locked_page);
    if (!wbc)
    continue;
    if (wbc->for_reclaim) {
    up(&inode->i_sem);
    up_read(&inode->i_sb->s_umount);
    }
    /* Was the page truncated under us? */
    if (page_mapping(locked_page) != mapping) {
    err = -ESTALE;
    goto err_out_locked;
    }
    } else {
    pages[nr] = find_lock_page(mapping, start);
    if (pages[nr])
    continue;
    if (!cached_page) {
    cached_page = alloc_page(gfp_mask);
    if (unlikely(!cached_page))
    goto err_out;
    }
    err = add_to_page_cache_lru(cached_page,
    mapping, start, gfp_mask);
    if (unlikely(err)) {
    if (err == -EEXIST)
    continue;
    goto err_out;
    }
    pages[nr] = cached_page;
    cached_page = NULL;
    }
    }

    The above fixes two bugs in the below:
    - if (!unlikely(cached_page)) should be if (unlikely(!cached_page))
    - The -EEXIST case after add_to_page_cache_lru() would result in
    an infinite loop in the original as nr wasn't being incremented.

    > + err = nr = 0;
    > + while (nr < nr_pages) {
    > + if (start == lp_idx) {
    > + pages[nr] = locked_page;
    > + if (nr) {
    > + lock_page(locked_page);
    > + if (wbc) {
    > + if (wbc->for_reclaim) {
    > + up(&inode->i_sem);
    > + up_read(&inode->i_sb->s_umount);
    > + }
    > + /* Was the page truncated under us? */
    > + if (page_mapping(locked_page) !=
    > + mapping) {
    > + err = -ESTALE;
    > + goto err_out_locked;
    > + }
    > + }
    > + }
    > + } else {
    > + pages[nr] = find_lock_page(mapping, start);
    > + if (!pages[nr]) {
    > + if (!cached_page) {
    > + cached_page = alloc_page(gfp_mask);
    > + if (!unlikely(cached_page))
    > + goto err_out;
    > + }
    > + err = add_to_page_cache_lru(cached_page,
    > + mapping, start, gfp_mask);
    > + if (unlikely(err)) {
    > + if (err == -EEXIST)
    > + continue;
    > + goto err_out;
    > + }
    > + pages[nr] = cached_page;
    > + cached_page = NULL;
    > + }
    > + }
    > + nr++;
    > + start++;
    > + }

    --
    "Next the statesmen will invent cheap lies, putting the blame upon
    the nation that is attacked, and every man will be glad of those
    conscience-soothing falsities, and will diligently study them, and refuse
    to examine any refutations of them; and thus he will by and by convince
    himself that the war is just, and will thank God for the better sleep
    he enjoys after this process of grotesque self-deception." -- Mark Twain
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:10    [W:0.024 / U:32.324 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site