一次向linux开源社区提交补丁的经历

背景

在开发过程当中，偶然发现了spinand驱动的一个bug，满怀欣喜地往社区提补丁。这是怎么样的一个bug呢？

static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
                            struct mtd_oob_ops *ops)
{
        ......
        nanddev_io_for_each_page(nand, from, ops, &iter) {
                ......
                ret = spinand_read_page(spinand, &iter.req, enable_ecc);
                if (ret < 0 && ret != -EBADMSG)     /* 读取数据出错 */
                        break;

                if (ret == -EBADMSG) {
                        /* -EBADMSG 返回表示坏块 */
                        ecc_failed = true;
                        mtd->ecc_stats.failed++;
                        ret = 0;
                } else {
                        /* 出现位翻转或者读取正常，则记录历史位翻转最大值 */
                        mtd->ecc_stats.corrected += ret;
                        max_bitflips = max_t(unsigned int, max_bitflips, ret);
                }

                ops->retlen += iter.req.datalen; 
                ops->oobretlen += iter.req.ooblen;
        }

        if (ecc_failed && !ret)
                ret = -EBADMSG;

        return ret ? ret : max_bitflips;
}

代码逻辑以下：

1. 遍历读取每个page
2. 若是读出错则直接返回
3. 若是出现坏块，则置位ecc_failed，在函数最后会检查此标志
4. 若是出现位翻转，则暂存最大位翻转的bit位数量
5. 所有读取完后，若是有置位ecc_failed，则返回坏块错误码；若是出现位翻转，则返回最大位翻转；不然返回0，表示正常

问题出在于，若是恰好最后一次读取出现位翻转，此时ret != 0就直接退出循环，此时会致使坏块标识无效，且返回最后的位翻转量而非历史位翻转最大值。这是代码不严谨的地方。

第一次提交

修改补丁以下，补丁逻辑再也不解释。

In function spinand_mtd_read, if the last page to read occurs bitflip,
this function will return error value because veriable ret not equal to 0.

Signed-off-by: liaoweixiong <liaoweixiong@allwinnertech.com>
---
 drivers/mtd/nand/spi/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 556bfdb..6b9388d 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -511,12 +511,12 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
        if (ret == -EBADMSG) {
            ecc_failed = true;
            mtd->ecc_stats.failed++;
-           ret = 0;
        } else {
            mtd->ecc_stats.corrected += ret;
            max_bitflips = max_t(unsigned int, max_bitflips, ret);
        }
 
+       ret = 0;
        ops->retlen += iter.req.datalen;
        ops->oobretlen += iter.req.ooblen;
    }

21:13分发出的邮件，21:45分陆续收到两个回复：

<maintainer A>:

Actually, that's exactly what the MTD core expects (see [1]), so you're
the one introducing a regression here.

<maintainer B>:

To me it looks like the patch description is somewhat incorrect, but the 
fix itself looks okay, unless I'm getting it wrong.

In case of the last page containing bitflips (ret > 0), 
spinand_mtd_read() will return that number of bitflips for the last 
page. But to me it looks like it should instead return max_bitflips like 
it does when the last page read returns with 0.

以及隔天回复

<maintainer A>:

Oh, you're right. liaoweixiong, can you adjust the commit message
accordingly?

好吧，问题出在与我没把问题描述清楚，改改再提交

第二次提交

只改了comment和补丁标题：

Subject: [PATCH v2] mtd: spinand: read return badly if the last page has bitflips

In case of the last page containing bitflips (ret > 0), 
spinand_mtd_read() will return that number of bitflips for the last 
page. But to me it looks like it should instead return max_bitflips like 
it does when the last page read returns with 0.

而后哗啦啦收到两个Reviewed-by，附带一个建议：

Reviewed-by: <maintainer B>

This should probably be resent with the following tags:

Cc: stable@vger.kernel.org
Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI 
NANDs")

得，再提交一次吧

第三次提交

此时的我提交补丁到社区经验并很少，Maintainer让我resend，我就忐忑开始胡思乱想了：

版本号须要累加么？该怎么标记是从新发送？有两个maintainer已经"承认"了个人补丁（reviewed-by），我改怎么体现到新的邮件中？

仔细想一想内容并没改，所以不须要累加版本号；查询前人提交，在邮件标题能够加上RESEND字样；搜索含RESEND字样的前人邮件，恰好找到一个在maintainer reviewed后resend为acked，写在signed-off-by区。

OK，肯定下来就从新发吧

Subject: [RESEND PATCH v2] mtd: spinand: read return badly if the last page has bitflips

......
Signed-off-by: liaoweixiong <liaoweixiong@allwinnertech.com>
Acked-by: <maintainer A>
Acked-by: <maintainer B>
Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI NANDs")

很快，就挨批了...

第四次提交

晚上10点多，收到回复：

<maintainer B>

Why did you change our Reviewed-by tags to Acked-by tags?

额...我也是看别人这么作我才这么作的，大佬生气了！赶忙补救

......
Reviewed-by: <maintainer A>
Reviewed-by: <maintainer B>
Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI NANDs")

第五次提交

埋下的坑终究是要踩的，很快，再次挨批了

<maintainer C>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

<maintainer A>

FYI, you should not send the patch to stable@vger.kernel.org, but 
instead, as I said in my other reply, add the tag "Cc: 
stable@vger.kernel.org". See "Option 1" in the document Greg referred to.

小白赶忙狠补基础操做规范...

第六次提交

......
Reviewed-by: <maintainer A>
Reviewed-by: <maintainer B>
Cc: stable@vger.kernel.org
Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI NANDs")

总结

哎，我只是挪了一行代码的位置而已啊，Maintainer严审下，我居然提交了6次！6次！忽然感受心好累。

累归累，问题总结仍是须要的

1. 新手不具有提交代码的一些常识，包括 a) 提交中各个tag的含义，在何时加这些tag，例如Reviewed-by和Acked-by的差异 b) 提交补丁到stable的注意事项
2. 对补丁的问题描述不够仔细清楚，致使maintainer B没法理解，幸亏maintainer A帮我澄清了

解决方法：

1. linux提交有规范文档的，抽时间撸一遍，并翻译发博客
2. 在发补丁以前，让身边的人帮忙看一下补丁说明是否足够清晰

但愿个人经历能帮助到正在或者准备向Linux内核开源社区的小伙伴

续：第七次提交

居然还要第七次提交，你敢相信？ 距离上一次提交过了2天，无声无息，而后一声惊雷，一个新的maintainer回复了

<maintainer D>

......
Please write your entire official first/last name(s)
......
Finally, when we ask you to resend a patch, it means sending a new
version of the patch. So in the subject, you should not use the
[RESEND] keyword (which means you are sending something again exactly
as it was before, you just got ignored, for example) but instead you
should increment the version number (v3) and also write a nice
changelog after the three dashes '---' (will be ignored by Git when
applying).

I would like to queue this for the next release so if you can do it
ASAP, that would be great.
.....

这邮件让我明白了4点：

1. 名字都要特定划分first/last name么？对署名都有要求...大佬要求，改！
2. Manintainer要求Resend，原来要累加版本号的，soga～
3. 在叠加版本时，须要在---的字段后添加版本迭代说明，跟以前发的系列补丁，在cover中说明还不同
4. RESEND 的关键字，表示以前的邮件被意外忽略了因此重发，明白了！

干起来！

Subject: [PATCH v3] mtd: spinand: read return badly if the last page has bitflips

......
Signed-off-by: Weixiong Liao <liaoweixiong@allwinnertech.com>
Reviewed-by: <maintainer A>
Reviewed-by: <maintainer B>
Cc: stable@vger.kernel.org
Fixes: 7529df465248 ("mtd: nand: Add core infrastructure to support SPI NANDs")
---
Changes since v2:
- Resend this patch with Cc and Fixes tags.

Changes since v1:
- More accurate description for this patch
---
......