Please Help: 2.6.37 a_ops->readpages is never called, 2.6.36 was fine
I was so busy with doing "write" development that I totally missed it!
Somewhere between 2.6.36 and 2.6.37 my exofs file system stopped to receive any a_ops->readpages() (with "s") calls it only receives the single page read, a_ops->readpage(). Needless to say that things get very slow.
In 2.6.36 I can clearly see both members getting called the plural one from read-ahead code.
I've reverted the few simple patches that went into 2.6.37 for exofs, but clearly it's not it, because the vector is just not called.
I'm now bisecting it, but in an hope that some kind soul can put me out of my misery.
So the question is:
Is there some new nob that needs to be set so read-head starts working again?
Thanks in Advance
Boaz
Re: Problem found, but why???
115e19c53501edc11f730191f7f047736815ae3d is the first bad commit commit 115e19c53501edc11f730191f7f047736815ae3d
Author: Boaz Harrosh <Boaz Harrosh [email protected]>
exofs: Set i_mapping->backing_dev_info anyway
OK It's stupid ME. But why please someone explain this issue? For now I'll send a revert, but I would like this explained, perhaps a more solid fix is needed.
Please look at this patch:
fs/exofs/inode.c +2 -0
@@ -1030,6 +1030,7 @@ struct inode *exofs_iget(struct super_block *sb, unsigned long ino)memcpy (oi->i_data, fcb.i_data,sizeof (fcb.i_data)); } + inode->i_mapping->backing_dev_info = sb->s_bdi;if (S_ISREG (inode->i_mode)) { inode->i_op = &exofs_file_inode_operations; inode->i_fop = &exofs_file_operations;
@@ -1129,6 +1130,7 @@ struct inode *exofs_new_inode(struct inode *dir, int mode) sbi = sb->s_fs_info; + inode->i_mapping->backing_dev_info = sb->s_bdi; sb->s_dirt =1 ;inode_init_owner (inode, dir, mode); inode->i_ino = sbi->s_nextid++;
All it does is set the inode's bdi to point to the super-block bdi. In stead of this default "null" bdi from VFS. When we do so the read-ahead stops coming, completely. Is there something that needs to be implemented farther for a super-block's bdi to support read-ahead? Please help?
Thanks
Boaz
Re: Problem found, but why???
Excerpts from Boaz Harrosh's message of 2011-02-02 13:46:52 -0500:
Quoted 9 lines
115e19c53501edc11f730191f7f047736815ae3dis the first bad commit commit115e19c53501edc11f730191f7f047736815ae3d
Author: Boaz Harrosh <Boaz Harrosh [email protected]>exofs: Set i_mapping->backing_dev_info anywayOK It's stupid ME. But why please someone explain this issue? For now I'll send a revert, but I would like this explained, perhaps a more solid fix is needed.
The bdi_init code doesn't seem to set the ra_pages to a default number. You got there via kzalloc, so the ra_pages field in your bdi is probably zero.
This should stop all readahead.
-chris
PATCH 1/1 exofs: Fix read ahead BUG, caused by moving inodes to sb->s_bdi
In reply to Boaz Harroshcommit: [115e19c] exofs: Set i_mapping->backing_dev_info anyway
Has caused a regression in read-ahead because bdi_init() does not set bdi->ra_pages to anything and it was left as zero. So when moving all inodes to point to the super-block's bdi, they all had ra_pages disabled.
Should bdi_init() be patched to some sane default value?
I fix that by calculating a read_ahead that is:
- preferable 2 stripes long (I'll add a mount option to next Kernel)
- Minimum 128K aligned up to stripe-size
- Caped to maximum-IO-sizes round down to stripe_size. (Max sizes are governed by max bio size that fits a page times
# of devices)
This is a regression since the 2.6.37 Kernel.
2.6.36 was fine
--- fs/exofs/exofs.h | 2 ++ fs/exofs/inode.c | 17 +++++++++++++---- fs/exofs/super.c | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-)
fs/exofs/exofs.h +2 -0
@@ -256,6 +256,8 @@ static inline int exofs_oi_read(struct exofs_i_info *oi, }/* inode.c */ +unsigned exofs_max_io_pages (struct exofs_layout *layout, +unsigned expected_pages);int exofs_setattr (struct dentry *,struct iattr *); int exofs_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned flags,
fs/exofs/inode.c +13 -4
@@ -43,6 +43,17 @@ enum { BIO_MAX_PAGES_KMALLOC = PAGE_SIZE / sizeof(struct page *), }; +unsigned exofs_max_io_pages (struct exofs_layout *layout, +unsigned expected_pages) +{ +unsigned pages =min_t (unsigned, expected_pages,MAX_PAGES_KMALLOC ); + +/* TODO: easily support bio chaining */ + pages =min_t (unsigned, pages, + layout->group_width *BIO_MAX_PAGES_KMALLOC ); +return pages; +} +struct page_collect {struct exofs_sb_info *sbi;struct inode *inode;
@@ -97,8 +108,7 @@ static void _pcol_reset(struct page_collect *pcol)static int pcol_try_alloc (struct page_collect *pcol) { -unsigned pages =min_t (unsigned, pcol->expected_pages, -MAX_PAGES_KMALLOC ); +unsigned pages;if (!pcol->ios) {/* First time allocate io_state */ int ret =exofs_get_io_state (&pcol->sbi->layout, &pcol->ios);
@@ -108,8 +118,7 @@ static int pcol_try_alloc(struct page_collect *pcol) }/* TODO: easily support bio chaining */ - pages =min_t (unsigned, pages, - pcol->sbi->layout.group_width *BIO_MAX_PAGES_KMALLOC ); + pages =exofs_max_io_pages (&pcol->sbi->layout, pcol->expected_pages);for (; pages; pages >>=1 ) { pcol->pages = kmalloc(pages * sizeof(struct page *),
fs/exofs/super.c +18 -0
@@ -383,6 +383,23 @@ static int _read_and_match_data_map(struct exofs_sb_info *sbi, unsigned numdevs,return 0 ; } +static unsigned __ra_pages (struct exofs_layout *layout) +{ +const unsigned _MIN_RA =32 ;/* min 128K read-ahead */ +unsigned ra_pages = layout->group_width * layout->stripe_unit / +PAGE_SIZE ; +unsigned max_io_pages =exofs_max_io_pages (layout, ~0 ); + + ra_pages *=2 ;/* two stripes */ +if (ra_pages < _MIN_RA) + ra_pages =roundup (_MIN_RA, ra_pages /2 ); + +if (ra_pages > max_io_pages) + ra_pages = max_io_pages; + +return ra_pages; +} +/* @odi is valid only as long as @fscb_dev is valid */ static int exofs_devs_2_odi (struct exofs_dt_device_info *dt_dev,struct osd_dev_info *odi)
@@ -616,6 +633,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent) }/* set up operation vectors */ + sbi->bdi.ra_pages =__ra_pages (&sbi->layout); sb->s_bdi = &sbi->bdi; sb->s_fs_info = sbi; sb->s_op = &exofs_sops;
Signature · 1 line
1.7.2.3
Re: [PATCH] exofs: Fix read ahead BUG, caused by moving inodes to sb->s_bdi
Quoted 15 lines · Boaz Harrosh
commit: [115e19c] exofs: Set
i_mapping->backing_dev_infoanywayHas caused a regression in read-ahead because
bdi_init()does not setbdi->ra_pagesto anything and it was left as zero. So when moving all inodes to point to the super-block's bdi, they all hadra_pagesdisabled.Should
bdi_init()be patched to some sane default value?I fix that by calculating a
read_aheadthat is:
- preferable 2 stripes long (I'll add a mount option to next Kernel)
- Minimum 128K aligned up to stripe-size
- Caped to maximum-IO-sizes round down tostripe_size. (Max sizes are governed by max bio size that fits a page times
Quoted 5 lines
# of devices)This is a regression since the 2.6.37 Kernel.
2.6.36 was fine
Dear Greg, Stable Tree.
I see that Linus has already merged the revert of the offending patch, as a BUG fix for above. That's fine. PLEASE forget about this patch. (Sorry for the noise)
I will submit this patch as a regular enhancement for the next merge window.
Thanks
Boaz
Quoted 99 lines
CC Stable Tree <[email protected]>Signed-off-by Boaz Harrosh <[email protected]>--- fs/exofs/exofs.h | 2 ++ fs/exofs/inode.c | 17 +++++++++++++---- fs/exofs/super.c | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-)fs/exofs/exofs.h +2
-0@@ -256,6 +256,8 @@ static inline int exofs_oi_read(struct exofs_i_info *oi, }/* inode.c */ +unsigned exofs_max_io_pages (struct exofs_layout *layout, +unsigned expected_pages);int exofs_setattr (struct dentry *,struct iattr *); int exofs_write_begin(struct file *file, struct address_space *mapping, loff_t pos, unsigned len, unsigned flags,fs/exofs/inode.c +13
-4@@ -43,6 +43,17 @@ enum { BIO_MAX_PAGES_KMALLOC = PAGE_SIZE / sizeof(struct page *), }; +unsigned exofs_max_io_pages (struct exofs_layout *layout, +unsigned expected_pages) +{ +unsigned pages =min_t (unsigned, expected_pages,MAX_PAGES_KMALLOC ); + +/* TODO: easily support bio chaining */ + pages =min_t (unsigned, pages, + layout->group_width *BIO_MAX_PAGES_KMALLOC ); +return pages; +} +struct page_collect {struct exofs_sb_info *sbi;struct inode *inode;@@ -97,8 +108,7 @@ static void _pcol_reset(struct page_collect *pcol)static int pcol_try_alloc (struct page_collect *pcol) { -unsigned pages =min_t (unsigned, pcol->expected_pages, -MAX_PAGES_KMALLOC ); +unsigned pages;if (!pcol->ios) {/* First time allocate io_state */ int ret =exofs_get_io_state (&pcol->sbi->layout, &pcol->ios);@@ -108,8 +118,7 @@ static int pcol_try_alloc(struct page_collect *pcol) }/* TODO: easily support bio chaining */ - pages =min_t (unsigned, pages, - pcol->sbi->layout.group_width *BIO_MAX_PAGES_KMALLOC ); + pages =exofs_max_io_pages (&pcol->sbi->layout, pcol->expected_pages);for (; pages; pages >>=1 ) { pcol->pages = kmalloc(pages * sizeof(struct page *),fs/exofs/super.c +18
-0@@ -383,6 +383,23 @@ static int _read_and_match_data_map(struct exofs_sb_info *sbi, unsigned numdevs,return 0 ; } +static unsigned __ra_pages (struct exofs_layout *layout) +{ +const unsigned _MIN_RA =32 ;/* min 128K read-ahead */ +unsigned ra_pages = layout->group_width * layout->stripe_unit / +PAGE_SIZE ; +unsigned max_io_pages =exofs_max_io_pages (layout, ~0 ); + + ra_pages *=2 ;/* two stripes */ +if (ra_pages < _MIN_RA) + ra_pages =roundup (_MIN_RA, ra_pages /2 ); + +if (ra_pages > max_io_pages) + ra_pages = max_io_pages; + +return ra_pages; +} +/* @odi is valid only as long as @fscb_dev is valid */ static int exofs_devs_2_odi (struct exofs_dt_device_info *dt_dev,struct osd_dev_info *odi)@@ -616,6 +633,7 @@ static int exofs_fill_super(struct super_block *sb, void *data, int silent) }/* set up operation vectors */ + sbi->bdi.ra_pages =__ra_pages (&sbi->layout); sb->s_bdi = &sbi->bdi; sb->s_fs_info = sbi; sb->s_op = &exofs_sops;