readahead: fix block ordering in __do_page_cache_readahead
Subsystems (3)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 readahead: fix block ordering in __do_page_cache_readahead
Hi,
I just ran into this today. I had temporarily disabled readpages in GFS2 in order to test something and discovered, to my surprise, that the block IOs were being issued in reverse order. In my case, the block_map function was called in reverse block order. In other words: lblock started at 8, then proceeded to 7, 6, 5, 4, 3, 2, 1, 0. So I surmised the order must be wrong. After a little digging, I whipped up this little patch. Can anyone out there corroborate this or tell me if I've I lost my mind?
Viro?
I'll add my Signed-off-by but I really haven't tested it or anything. It's kind of moot for file systems with readpages, but it might be worth doing.
Regards,
Bob Peterson
Red Hat File Systems
---
mm/readahead.c +1 -1
@@ -185,7 +185,7 @@ int __do_page_cache_readahead(struct address_space *mapping, struct file *filp,if (!page)break ; page->index = page_offset; -list_add (&page->lru, &page_pool); +list_add_tail (&page->lru, &page_pool);if (page_idx == nr_to_read - lookahead_size)SetPageReadahead (page); ret++;
Jan Kara
maintainerQuoted 9 lines · Bob Peterson
Hi,
I just ran into this today. I had temporarily disabled readpages in GFS2 in order to test something and discovered, to my surprise, that the block IOs were being issued in reverse order. In my case, the
block_mapfunction was called in reverse block order. In other words: lblock started at 8, then proceeded to 7, 6, 5, 4, 3, 2, 1, 0. So I surmised the order must be wrong. After a little digging, I whipped up this little patch. Can anyone out there corroborate this or tell me if I've I lost my mind?
Quoted 6 lines
Viro?
I'll add my Signed-off-by but I really haven't tested it or anything. It's kind of moot for file systems with readpages, but it might be worth doing.
Signed-off-by Bob Peterson <[email protected]>
The patch looks good to me. But I've added CC's to some relevant people.
Honza
Quoted 18 lines
---
mm/readahead.c +1
-2@@ -185,7 +185,7 @@ int __do_page_cache_readahead(struct address_space *mapping, struct file *filp,if (!page)break ; page->index = page_offset; -list_add (&page->lru, &page_pool); +list_add_tail (&page->lru, &page_pool);if (page_idx == nr_to_read - lookahead_size)SetPageReadahead (page); ret++; --To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Andrew Morton
maintainerQuoted 10 lines · On Thu, 21 Jan 2016 15:09:16 +0100 Jan Kara <[email protected]> wrote:
Quoted 9 lines · Bob Peterson
Hi,
I just ran into this today. I had temporarily disabled readpages in GFS2 in order to test something and discovered, to my surprise, that the block IOs were being issued in reverse order. In my case, the
block_mapfunction was called in reverse block order. In other words: lblock started at 8, then proceeded to 7, 6, 5, 4, 3, 2, 1, 0. So I surmised the order must be wrong. After a little digging, I whipped up this little patch. Can anyone out there corroborate this or tell me if I've I lost my mind?
Quoted 9 lines
Quoted 6 lines
Viro?
I'll add my Signed-off-by but I really haven't tested it or anything. It's kind of moot for file systems with readpages, but it might be worth doing.
Signed-off-by Bob Peterson <[email protected]>The patch looks good to me. But I've added CC's to some relevant people.
The patch looks good to me. How curious. It's hard to believe that this has persisted for so long without being noticed.
However "kernel test robot" is reporting a 13.5% performance regression from this patch. http://comments.gmane.org/gmane.linux.kernel/2128714