Christoph Lameter <[email protected]> wrote:
>
> +int buffer_migrate_page(struct page *newpage, struct page *page)
> +{
> + struct address_space *mapping = page->mapping;
> + struct buffer_head *bh, *head;
> +
> + if (!mapping)
> + return -EAGAIN;
> +
> + if (!page_has_buffers(page))
> + return migrate_page(newpage, page);
> +
> + head = page_buffers(page);
> +
> + if (migrate_page_remove_references(newpage, page, 3))
> + return -EAGAIN;
> +
> + spin_lock(&mapping->private_lock);
Why are you taking ->private_lock here?
address_space.private_lock protects the list of buffers at
address_space.private_list. For a regular file (or directory or long
symlink..) that list contains buffers against the blockdev mapping (a
different address_space) which need to be synced for a successful fsync of
this file. ie: dirty metadata for this file.
So we have two situations:
a) page->mapping->host refers to a regular file/dir/etc
Here, mapping->private_list holds potentially-dirty buffers against
the blockdev mapping (a different address_space).
Nothing needs to be done.
b) page->mapping->host refers to a blockdev (/dev/hda1's pages)
Here, mapping->private_list is actually always empty.
Nothing needs to be done.
BUT, page_buffers(page) refers to buffers which might be on some
other address_space's ->private_list. Because a blockdev may have dirty
buffers which some other address_space needs to write out for its sync.
blockdevmapping->private_lock is the correct lock for these buffers.
Each regular file has a copy of blockdevmapping in its ->assoc_mapping,
so all files end up taking the same lock when manipulating their
->private_list.
As long as you've taken a ref on the blockdev mapping's buffers and
locked them then nobody will be starting I/O against them or fiddling
with ->b_page while you do the swizzle (I think).
AFAIK nobody ever used address_space.private_list for anything apart from
the associated buffers, but that's just a btw.
Anyway, ->private_lock is purely for protecting the thing at
->private_list, so I suspect this locking is simply unneeded.
Please explain the reasoning behind taking this lock. In fact, that should
have been commented, in the spirit of buffer.c's glorious overcommenting,
which I'm sure you enjoyed ;)
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
