When a buffer is added to the LRU list, a reference is taken which is not dropped until the buffer is evicted from the LRU list. This is the correct behavior, however this LRU reference will prevent the buffer from being dropped. This means that the buffer can't actually be dropped until it is selected for eviction. There's no bound on the time spent on the LRU list, which means that the buffer may be undroppable for very long periods of time. Given that migration involves dropping buffers, the associated page is now unmigratible for long periods of time as well. CMA relies on being able to migrate a specific range of pages, so these these types of failures make CMA significantly less reliable, especially under high filesystem usage.
Rather than waiting for the LRU algorithm to eventually kick out the buffer, explicitly remove the buffer from the LRU list when trying to drop it. There is still the possibility that the buffer could be added back on the list, but that indicates the buffer is still in use and would probably have other 'in use' indicates to prevent dropping.
Signed-off-by: Laura Abbott lauraa@codeaurora.org --- fs/buffer.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/fs/buffer.c b/fs/buffer.c index ad5938c..daa0c3d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1399,12 +1399,49 @@ static bool has_bh_in_lru(int cpu, void *dummy) return 0; }
+static void __evict_bh_lru(void *arg) +{ + struct bh_lru *b = &get_cpu_var(bh_lrus); + struct buffer_head *bh = arg; + int i; + + for (i = 0; i < BH_LRU_SIZE; i++) { + if (b->bhs[i] == bh) { + brelse(b->bhs[i]); + b->bhs[i] = NULL; + goto out; + } + } +out: + put_cpu_var(bh_lrus); +} + +static bool bh_exists_in_lru(int cpu, void *arg) +{ + struct bh_lru *b = per_cpu_ptr(&bh_lrus, cpu); + struct buffer_head *bh = arg; + int i; + + for (i = 0; i < BH_LRU_SIZE; i++) { + if (b->bhs[i] == bh) + return 1; + } + + return 0; + +} void invalidate_bh_lrus(void) { on_each_cpu_cond(has_bh_in_lru, invalidate_bh_lru, NULL, 1, GFP_KERNEL); } EXPORT_SYMBOL_GPL(invalidate_bh_lrus);
+void evict_bh_lrus(struct buffer_head *bh) +{ + on_each_cpu_cond(bh_exists_in_lru, __evict_bh_lru, bh, 1, GFP_ATOMIC); +} +EXPORT_SYMBOL_GPL(evict_bh_lrus); + void set_bh_page(struct buffer_head *bh, struct page *page, unsigned long offset) { @@ -3052,6 +3089,7 @@ drop_buffers(struct page *page, struct buffer_head **buffers_to_free)
bh = head; do { + evict_bh_lrus(bh); if (buffer_write_io_error(bh) && page->mapping) set_bit(AS_EIO, &page->mapping->flags); if (buffer_busy(bh))
On Tue, 11 Sep 2012, Laura Abbott wrote:
When a buffer is added to the LRU list, a reference is taken which is not dropped until the buffer is evicted from the LRU list. This is the correct behavior, however this LRU reference will prevent the buffer from being dropped. This means that the buffer can't actually be dropped until it is selected for eviction. There's no bound on the time spent on the LRU list, which means that the buffer may be undroppable for very long periods of time. Given that migration involves dropping buffers, the associated page is now unmigratible for long periods of time as well.
Disclaimer: I'm no expert on buffer_heads, and haven't studied your patch. But it seems to me that this is an issue with the (unnamed) filesystem you use, rather than a problem to be solved in drop_buffers().
extN, gfs2, ntfs, ocfs2 and xfs set .migratepage = buffer_migrate_page, and I cannot see that page migration involves drop_buffers() at all in that case: it transfers the buffer_heads from the old page to the new, whether they're busy or not, with no attempt to free them.
Maybe your filesystem can be converted, with or without some extra help, to buffer_migrate_page() instead of the default fallback_migrate_page(): which indeed has to play safe, doing the try_to_release_page() you see. Maybe ask on the mailing list for your filesystem?
Hugh
CMA relies on being able to migrate a specific range of pages, so these these types of failures make CMA significantly less reliable, especially under high filesystem usage.
Rather than waiting for the LRU algorithm to eventually kick out the buffer, explicitly remove the buffer from the LRU list when trying to drop it. There is still the possibility that the buffer could be added back on the list, but that indicates the buffer is still in use and would probably have other 'in use' indicates to prevent dropping.
Signed-off-by: Laura Abbott lauraa@codeaurora.org
fs/buffer.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/fs/buffer.c b/fs/buffer.c index ad5938c..daa0c3d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -1399,12 +1399,49 @@ static bool has_bh_in_lru(int cpu, void *dummy) return 0; } +static void __evict_bh_lru(void *arg) +{
- struct bh_lru *b = &get_cpu_var(bh_lrus);
- struct buffer_head *bh = arg;
- int i;
- for (i = 0; i < BH_LRU_SIZE; i++) {
if (b->bhs[i] == bh) {brelse(b->bhs[i]);b->bhs[i] = NULL;goto out;}- }
+out:
- put_cpu_var(bh_lrus);
+}
+static bool bh_exists_in_lru(int cpu, void *arg) +{
- struct bh_lru *b = per_cpu_ptr(&bh_lrus, cpu);
- struct buffer_head *bh = arg;
- int i;
- for (i = 0; i < BH_LRU_SIZE; i++) {
if (b->bhs[i] == bh)return 1;- }
- return 0;
+} void invalidate_bh_lrus(void) { on_each_cpu_cond(has_bh_in_lru, invalidate_bh_lru, NULL, 1, GFP_KERNEL); } EXPORT_SYMBOL_GPL(invalidate_bh_lrus); +void evict_bh_lrus(struct buffer_head *bh) +{
- on_each_cpu_cond(bh_exists_in_lru, __evict_bh_lru, bh, 1, GFP_ATOMIC);
+} +EXPORT_SYMBOL_GPL(evict_bh_lrus);
void set_bh_page(struct buffer_head *bh, struct page *page, unsigned long offset) { @@ -3052,6 +3089,7 @@ drop_buffers(struct page *page, struct buffer_head **buffers_to_free) bh = head; do {
evict_bh_lrus(bh);-- 1.7.11.3
Hi,
On 9/14/2012 6:41 PM, Hugh Dickins wrote:
On Tue, 11 Sep 2012, Laura Abbott wrote:
When a buffer is added to the LRU list, a reference is taken which is not dropped until the buffer is evicted from the LRU list. This is the correct behavior, however this LRU reference will prevent the buffer from being dropped. This means that the buffer can't actually be dropped until it is selected for eviction. There's no bound on the time spent on the LRU list, which means that the buffer may be undroppable for very long periods of time. Given that migration involves dropping buffers, the associated page is now unmigratible for long periods of time as well.
Disclaimer: I'm no expert on buffer_heads, and haven't studied your patch. But it seems to me that this is an issue with the (unnamed) filesystem you use, rather than a problem to be solved in drop_buffers().
We are using ext4
extN, gfs2, ntfs, ocfs2 and xfs set .migratepage = buffer_migrate_page, and I cannot see that page migration involves drop_buffers() at all in that case: it transfers the buffer_heads from the old page to the new, whether they're busy or not, with no attempt to free them.
That's true for most of the address spaces EXCEPT for the journaled address space operations; ext4_ordered_aops, ext4_writeback_aops, ext4_da_aops all set migratepage but ext4_journalled_aops does not set migratepage at all. This seems to be true all the way back to when the migratepage was added for ext3.
Maybe your filesystem can be converted, with or without some extra help, to buffer_migrate_page() instead of the default fallback_migrate_page(): which indeed has to play safe, doing the try_to_release_page() you see. Maybe ask on the mailing list for your filesystem?
I could ask on the ext mailing list for the historical reasons why the journalled ops don't have migrate pages, but I'm still going to assert this is still a problem with fallback_migrate_page. It's still possible to have drop_buffers fail unnecessarily because the buffer is stuck on the LRU list and I don't see why the problem shouldn't be fixed there as well.
Hugh
Thanks, Laura
linaro-mm-sig@lists.linaro.org
-
Hugh Dickins -
Laura Abbott