On Sunday 16 March 2008, Peter Teoh wrote: > In many parts of code map_extent_buffer() return value is not handled. > What is this function doing? Essentially calling kmap_atomic() > using the KM_USER1 memory, right? This memory is only one page in > size, and so it may failed easily? > > map_extent_buffer(parent, > btrfs_node_key_ptr_offset(i), > sizeof(struct btrfs_key_ptr), > &parent->map_token, &parent->kaddr, > &parent->map_start, > &parent->map_len, KM_USER1); > > Personally, I don't think this API is wise to use kmap_atomic(), as in > many parts of kernel codes, in between kmap_ and kunmap_ it is just a > few instructions from each other. But here I can see like this: >
kmap_atomic is used because the page may be a highmem page. We are required to do extra steps to access the contents of the page, but it allows us to use any page on the system as opposed to just the first 1GB of lowmem pages (on i386 at least). x86-64 doesn't have such restrictions and kmap atomic is inexpensive there (but not entirely free). map_extent_buffer has a number of different uses. It caches mappings so that repeated calls can be done by all of the btrfs_set/get functions in tight loops. If it is called with an existing mapping, the old mapping is unmapped before it continues. It can return an error, but only when it is called incorrectly. Older versions had BUG_ONs and WARN_ONs in various places to find those errors. (Yes, there are a number of places in btrfs right now that need proper error checking. The code churn is very high right now, and I'm focusing on other bits at the moment). -chris _______________________________________________ Btrfs-devel mailing list [email protected] http://oss.oracle.com/mailman/listinfo/btrfs-devel
