我们都知道linux内核中的代码非常精妙,但是有些代码由于历史原因,一个函数为了兼容处理各种情况,到最后可能变得非常糟糕,到处都是goto 和if,看的想跳楼(宋老师口头禅^_^)


如果在系统中读一个文件时会调用

generic_file_buffered_read

这个函数的功能是把磁盘中的数据读到page之后,或者直接获取cache中的page,然后调用copy_page_to_iter把page拷贝到用户层的buffer中。


一天寂静的下午,得空,打开电脑,准备仔细研究一下这个函数,发现这个函数的注释上面就写明了:

 * This is really ugly. But the goto's actually try to clarify some * of the logic when it comes to error handling etc.


仔细看了一下代码,果然ugly的不像话,到处都是跳转和判断,令人眩晕,而且整个函数达到300行左右(原谅我看了注释才斗胆这样讲:-)  ),发现要是把这个函数看下去,今天一整天的心情都不会好了(当时看的是Linux5.10的代码)


ssize_t generic_file_buffered_read(struct kiocb *iocb,                struct iov_iter *iter, ssize_t written){
find_page:                if (fatal_signal_pending(current)) {                        error = -EINTR;                        goto out;                }                        error = wait_on_page_locked_killable(page);                        if (unlikely(error))                                goto readpage_error;                        if (PageUptodate(page))                                goto page_ok;
                       if (inode->i_blkbits == PAGE_SHIFT ||                                        !mapping->a_ops->is_partially_uptodate)                                goto page_not_up_to_date;                        /* pipes can't handle partially uptodate pages */                        if (unlikely(iov_iter_is_pipe(iter)))                                goto page_not_up_to_date;                        if (!trylock_page(page))                                goto page_not_up_to_date;                        /* Did it get truncated before we got the lock? */                        if (!page->mapping)                                goto page_not_up_to_date_locked;                        if (!mapping->a_ops->is_partially_uptodate(page,                                                        offset, iter->count))                                goto page_not_up_to_date_locked;                        unlock_page(page);                }


于是就想内核社区这么多牛人,他们整天盯着这些代码,肯定很多人早已经注意到了,于是想去看看有没有人提交patch重构这个函数:


./scripts/get_maintainer.pl   mm/filemap.clinux-kernel@vger.kernel.org (open list)


然后我就在下面网址中搜索generic_file_buffered_read,果然在10月25号(我看代码那天在11月1号前后),就有人发了相关patch:


https://lore.kernel.org/lkml/




然后迫不及待查看patch,并把整个patch 下载下来:


这里推荐一个工具,使用b4工具

https://git.kernel.org/pub/scm/utils/b4/b4.git

可以直接从

https://lore.kernel.org

获取原始格式的patch,便于自己git am之后测试。

# b4 am https://lore.kernel.org/lkml/20201025212949.602194-1-kent.overstreet@gmail.comv2_20201025_kent_overstreet_generic_file_buffered_read_improvements.coverv2_20201025_kent_overstreet_generic_file_buffered_read_improvements.mbx

然后直接 git am ,非常方便,这样就打上了lore.kernel.org上提交的patch.

git am v2_20201025_kent_overstreet_generic_file_buffered_read_improvements.mbx提示:在git am之前,可以提前git apply --check 一下


# gitlogdate -3fc5608fc9917    2020-10-25      Kent Overstreet fs: generic_file_buffered_read() now uses find_get_pages_contig3bcadc3306be    2020-10-25      Kent Overstreet fs: Break generic_file_buffered_read up into multiple functions3650b228f83a    2020-10-25      Linus Torvalds  Linux 5.10-rc1
alias gitlogdate='git log --pretty=format:"%h%x09%ad%x09%an%x09%s" --date=short'


打了这个patch之后,generic_file_buffered_read变成了这个样子:


ssize_t generic_file_buffered_read(struct kiocb *iocb,                struct iov_iter *iter, ssize_t written){..pg_nr = generic_file_buffered_read_get_pages(iocb, iter,                                 pages, nr_pages);                                 ...for (i = 0; i < pg_nr; i++) {   copied = copy_page_to_iter(pages[i], offset, bytes, iter);   }


而且

generic_file_buffered_read_get_pages


也非常之清晰:

static int generic_file_buffered_read_get_pages(struct kiocb *iocb,                                                struct iov_iter *iter,                                                struct page **pages,                                                unsigned int nr)        nr_got = find_get_pages_contig(mapping, index, nr, pages);        if (nr_got)                goto got_pages;
       if (iocb->ki_flags & IOCB_NOIO)                return -EAGAIN;
       page_cache_sync_readahead(mapping, ra, filp, index, last_index - index);
       nr_got = find_get_pages_contig(mapping, index, nr, pages);        if (nr_got)                goto got_pages;...}


看完generic_file_buffered_read()之后,整个下午的心情都好多了。


ps: 这个补丁很快就被各种review, 并被相关maintainer收进自己仓库。