Phillip Lougher <[EMAIL PROTECTED]> wrote: > > Andrew Morton wrote in relation to my initial SquashFS patch: > > Phillip Lougher <[EMAIL PROTECTED]> wrote: > > > >>+skip_read: > >>+ memset(pageaddr + bytes, 0, PAGE_CACHE_SIZE - bytes); > >>+ kunmap(page); > >>+ flush_dcache_page(page); > >>+ SetPageUptodate(page); > >>+ unlock_page(page); > >>+ > >>+ return 0; > >>+} > > > See if you can use kmap_atomic() here - kmap() is slow, theoretically > > deadlocky and is deprecated. > > > > From some browsing of the kernel source and a useful article on lwn.net > I think I can replace all the readpage_xxx (symlink, readpage & > readpage4k) kmap/kunmaps with kmap_atomic(page, KM_USER0)/kunmap(kaddr, > KM_USER0)...
Yes. Generally the only reason the kernel needs to modify pagecache pages is to do a quick memset or to copy a string in there or something like that. So a quick kmap_atomic/memory copy/kunmap_atomic() cycle is a fairly common pattern. The one exception is the directory-in-pagecache code where we tend to do a lot of stuff while holding the kmap. Not that there's any particular reason why we needed to do it that way. > The lwn.net article mentions that k[un]map_atomic is the new alternative > to kmap (as Andrew Morton mentioned). > > I've noticed in asm-ppc/highmem.h the following comment > > /* > * The use of kmap_atomic/kunmap_atomic is discouraged - kmap/kunmap > * gives a more generic (and caching) interface. But kmap_atomic can > * be used in IRQ contexts, so in some (very limited) cases we need > * it. > */ > > Given what Andrew and the lwn.net article says, this comment is rather > confusing. Is it wrong? I'd say so, yes. For a start, kmap_high() takes a global lock. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
