Hi Chao, On 2018/8/10 11:05, Chao Yu wrote: > On 2018/8/1 14:01, Gao Xiang wrote: >> This patch seperates 'erofs_get_meta_page' into 'erofs_get_meta_page' >> and 'erofs_get_meta_page_nofail'. The second one ensures it should not >> fail due to memory pressure. >> >> It also adds const variables in order to fulfill 80 character limit. >> >> Signed-off-by: Gao Xiang <[email protected]> >> --- >> drivers/staging/erofs/data.c | 45 >> +++++++++++++++++++++++---------------- >> drivers/staging/erofs/internal.h | 24 ++++++++++++++++----- >> drivers/staging/erofs/unzip_vle.c | 12 ++++++----- >> drivers/staging/erofs/xattr.c | 23 ++++++++++++-------- >> 4 files changed, 67 insertions(+), 37 deletions(-) >> >> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c >> index 2426eda..e8637d6 100644 >> --- a/drivers/staging/erofs/data.c >> +++ b/drivers/staging/erofs/data.c >> @@ -39,31 +39,42 @@ static inline void read_endio(struct bio *bio) >> } >> >> /* prio -- true is used for dir */ >> -struct page *erofs_get_meta_page(struct super_block *sb, >> - erofs_blk_t blkaddr, bool prio) >> +struct page *__erofs_get_meta_page(struct super_block *sb, >> + erofs_blk_t blkaddr, bool prio, bool nofail) >> { >> - struct inode *bd_inode = sb->s_bdev->bd_inode; >> - struct address_space *mapping = bd_inode->i_mapping; >> + struct inode *const bd_inode = sb->s_bdev->bd_inode; >> + struct address_space *const mapping = bd_inode->i_mapping; >> + /* prefer retrying in the allocator to blindly looping below */ >> + const gfp_t gfp = mapping_gfp_constraint(mapping, ~__GFP_FS) | >> + (nofail ? __GFP_NOFAIL : 0); >> struct page *page; >> >> repeat: >> - page = find_or_create_page(mapping, blkaddr, >> - /* >> - * Prefer looping in the allocator rather than here, >> - * at least that code knows what it's doing. >> - */ >> - mapping_gfp_constraint(mapping, ~__GFP_FS) | __GFP_NOFAIL); >> - >> - BUG_ON(!page || !PageLocked(page)); >> + page = find_or_create_page(mapping, blkaddr, gfp); >> + if (unlikely(page == NULL)) { >> + DBG_BUGON(nofail); >> + return ERR_PTR(-ENOMEM); >> + } >> >> if (!PageUptodate(page)) { >> struct bio *bio; >> int err; >> >> - bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, true); >> + bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, nofail); >> + if (unlikely(bio == NULL)) { >> + DBG_BUGON(nofail); >> + err = -ENOMEM; >> +err_out: >> + unlock_page(page); >> + put_page(page); >> + return ERR_PTR(err); >> + } >> >> err = bio_add_page(bio, page, PAGE_SIZE, 0); >> - BUG_ON(err != PAGE_SIZE); >> + if (unlikely(err != PAGE_SIZE)) { >> + err = -EFAULT; >> + goto err_out; >> + } >> >> __submit_bio(bio, REQ_OP_READ, >> REQ_META | (prio ? REQ_PRIO : 0)); >> @@ -79,10 +90,8 @@ struct page *erofs_get_meta_page(struct super_block *sb, >> >> /* more likely a read error */ >> if (unlikely(!PageUptodate(page))) { >> - unlock_page(page); >> - put_page(page); >> - >> - page = ERR_PTR(-EIO); > DBG_BUGON(nofail); > > Thanks, >
nofail in erofs means "no fail under memory pressure", if !PageUptodate(page) after endio, it means "a IO read error occurred", so I think it could happen on disk error and it is not suitable to guarantee disk error nofail (...may be cause EIO again and again...)....... Thanks, Gao Xiang
