lkml.org 
[lkml]   [2007]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Pagecache zeroing: zero_user_segment, zero_user_segments and zero_user
    On 5/16/07, Andrew Morton <akpm@linux-foundation.org> wrote:
    > >
    > > 2. It has a useless parameter specifying the kmap slot.
    > > The functions above default to KM_USER0 which is also always used when
    > > zero_user_page was called except in one single case. We open code that
    > > single case to draw attention to the spot.
    > >
    >
    > Dunno. fwiw, we decided to _not_ embed KM_USER0 in the callee: we have had
    > some pretty ghastly bugs in the past due to misuse of kmap slots so the
    > idea was to shove the decision into the caller's face, make them think
    > about what they're doing

    Christoph hasn't really _replaced_ any !KM_USER0 case with
    KM_USER0 in this patch. AFAIR, we had originally decided to not
    embed KM_USER0 in zero_user_page because of this complaint
    from Anton Altaparmakov:
    http://lkml.org/lkml/2007/4/10/62

    But Christoph's patch here seems to have left that fs/ntfs/aops.c case
    alone, and used the zero_xxx helpers only for KM_USER0 cases, so
    we should probably be safe.

    > > +static inline void zero_user_segments(struct page *page,
    > > + unsigned start1, unsigned end1,
    > > + unsigned start2, unsigned end2)
    > > +{
    > > + void *kaddr = kmap_atomic(page, KM_USER0);
    > > +
    > > + BUG_ON(end1 > PAGE_SIZE ||
    > > + end2 > PAGE_SIZE);
    > > +
    > > + if (end1 > start1)
    > > + memset(kaddr + start1, 0, end1 - start1);
    > > +
    > > + if (end2 > start2)
    > > + memset(kaddr + start2, 0, end2 - start2);
    > > +
    > > + flush_dcache_page(page);
    > > + kunmap_atomic(kaddr, KM_USER0);
    > > +}
    >
    > For some reason we've always done the flush_dcache_page() while holding the
    > kmap. There's no reason for doing it this way and it just serves to worsen
    > scheduling latency a tiny bit.

    Wonder if any arch requires a valid mapping when doing the flush?

    (reading Documentation/cachetlb.txt)

    Documentation/cachetlb.txt is silent on whether flush_dcache_page()
    really must be called before a kunmap(), but mentions that
    flush_kernel_dcache_page() _must_ be called _before_ kunmap.
    And I saw that parisc implements flush_dcache_page() using
    flush_kernel_dcache_page itself (but then, we've not implemented a
    meaningful kmap for parisc in the first place), so we've _just_
    managed to be fine here ...

    Still, flush_dcache_page() before kunmap'ing is an old practice,
    would be wise to get this whetted by linux-arch first?

    Satyam
    -
    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: 2007-05-16 19:41    [W:0.035 / U:30.136 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site