一次向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
---
......