メカトラックス株式会社の @m_honda です。
今回は linux kernel のバグを修正するパッチを作成してメーリングリストに投稿したときのお話です。
はじめに
メカトラックス株式会社では Raspberry Pi 向けの機能拡張基板を製造販売しています。
製品のひとつに ADPi Pro と呼ばれる A/D 変換基板があります。
ADPi Pro は linux の kernel モジュールを使用することで、iio subsystem を利用した低レイテンシ動作が可能です。
しかし、あるときから電圧が正しく読み取れなくなりました。
調べてみたところ、どうやら kernel モジュールにバグがあるようでした。
バグを修正できたようだったので、linux kernel の mainline に取り込んでもらうことにしました。
作業は Submitting patches: the essential guide to getting your code into the kernel を参考にしました。
ここでは、振り返りを兼ねてドキュメントに沿って作業内容を見ていきたいと思います。
あらためて、レビュアーを務めていただいた Nuno Sá 氏、メンテナーの Jonathan Cameron 氏に感謝します。
Obtain a current source tree
mainline kernel のソースを取得します。
時間がかかるので気長に待ちましょう。
修正すべきコミットが分かっている場合には、必要な部分だけ shallow clone しても良いでしょう。
git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Describe your changes
ファイルを修正し、コミットします。
コミットする際に -s オプションをつけると Signed-off-by: タグが追加されます。
git commit -s -v
次のコマンドを実行することで、Signed-off-by: タグをデフォルトで追加するように設定できます。1
git config format.signOff yes
特定のコミットにバグが含まれている場合には、Fixes: タグを使ってコミットハッシュを記述します。
次のコマンドを実行して、git log や git show の出力を変更できるようにしておくと便利です。
git config --add core.abbrev 12
git config --add pretty.fixes "Fixes: %h (\"%s\")"
たとえば、次のように表示されます。
$ git log -1 --pretty=fixes 1a913270e57
Fixes: 1a913270e57a ("iio: adc: ad7793: Fix IRQ flag")
Separate your changes
変更点が少なく機能的にも単一なので分割しなくてもよさそうです。
ここはスキップしました。
Style-check your changes
次のコマンドを実行することでコーディングスタイルのチェックができます。
git diff HEAD^ | scripts/checkpatch.pl
実行結果は次のようになりました。
$ git diff HEAD^ | scripts/checkpatch.pl
total: 0 errors, 0 warnings, 7 lines checked
Your patch has no obvious style problems and is ready for submission.
問題は無さそうです。
Select the recipients for your patch
./scripts/get_maintainer.pl の引数にファイルのパスを渡してメンテナー情報を取得します。
$ ./scripts/get_maintainer.pl drivers/iio/adc/ad_sigma_delta.c
Lars-Peter Clausen <lars@********.***> (supporter:ANALOG DEVICES INC IIO DRIVERS)
Michael Hennerich <Michael.Hennerich@********.***> (supporter:ANALOG DEVICES INC IIO DRIVERS)
Jonathan Cameron <jic23@********.***> (maintainer:IIO SUBSYSTEM AND DRIVERS)
linux-iio@********.*** (open list:IIO SUBSYSTEM AND DRIVERS)
linux-kernel@********.*** (open list)
（メールアドレスの一部を * で伏せていますが、実際にはきちんと表示されます）
No MIME, no links, no compression, no attachments. Just plain text
メールの送信には推奨されている git send-email を使用しました。
チュートリアルもあり、実際にメールの送受信の練習ができます。
（筆者はいきなりメーリングリストに投稿する自信はなかったので、まずチュートリアルをこなしました）
--to オプションや --cc オプションを使って送信先を指定します。
今回はチュートリアルのようにあらかじめ to と cc を git config で設定しておきました。
これならタイプミスをすることはありません。
git config sendemail.to "$(git diff HEAD^ | ./scripts/get_maintainer.pl --separator , --nokeywords --nogit --nogit-fallback --norolestats --nol)"
git config sendemail.cc "$(git diff HEAD^ | ./scripts/get_maintainer.pl --separator , --nokeywords --nogit --nogit-fallback --norolestats --nom)"
tocmd, cccmd を設定する方法もあります。 2
次のコマンドを実行してメールを送信します。
git send-email HEAD^
コマンドを実行すると、次のようなメッセージが表示されました。
/tmp/KyEzGM2nUU/0001-Fix-IRQ-issue-by-setting-IRQ_DISABLE_UNLAZY-flag.patch
(mbox) Adding cc: Masahiro Honda <honda@********.***> from line 'From: Masahiro Honda <honda@********.***>'
(body) Adding cc: Masahiro Honda <honda@********.***> from line 'Signed-off-by: Masahiro Honda <honda@********.***>'
From: Masahiro Honda <honda@********.***>
To: Lars-Peter Clausen <lars@********.***>,
Michael Hennerich <Michael.Hennerich@********.***>,
Jonathan Cameron <jic23@********.***>
Cc: linux-iio@********.***,
linux-kernel@********.***,
Masahiro Honda <honda@********.***>
Subject: [PATCH] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag
Date: Mon, 6 Mar 2023 13:47:37 +0900
Message-Id: <20230306044737.862-1-honda@********.***>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
The Cc list above has been expanded by additional
addresses found in the patch commit message. By default
send-email prompts before sending whenever this occurs.
This behavior is controlled by the sendemail.confirm
configuration setting.
For additional information, run 'git send-email --help'.
To retain the current behavior, but squelch this message,
run 'git config --global sendemail.confirm auto'.
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll):
y を入力してエンターを押します。
OK. Log says:
Sendmail: /home/honda/go/bin/sendgmail -sender=honda@********.*** -i lars@********.*** Michael.Hennerich@********.*** jic23@********.*** linux-iio@********.*** linux-kernel@********.*** honda@********.***
From: Masahiro Honda <honda@********.***>
To: Lars-Peter Clausen <lars@********.***>,
Michael Hennerich <Michael.Hennerich@********.***>,
Jonathan Cameron <jic23@********.***>
Cc: linux-iio@********.***,
linux-kernel@********.***,
Masahiro Honda <honda@********.***>
Subject: [PATCH] Fix IRQ issue by setting IRQ_DISABLE_UNLAZY flag
Date: Mon, 6 Mar 2023 13:47:37 +0900
Message-Id: <20230306044737.862-1-honda@********.***>
X-Mailer: git-send-email 2.34.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: OK
結果は OK でした。
これで無事にパッチが投稿されているはずです。
Respond to review comments
レビュアーの方から返信をもらった際の心がけが書かれています。
筆者は最初に Nuno 氏から返信をもらいました。
氏にはコミットメッセージの指摘、バグの詳細やパッチの内容についての意見をもらうなどたいへんお世話になりました。
Use trimmed interleaved replies in email discussions
メールはインラインでの返信が原則とされています。
筆者は普段の業務では全文引用で返信することが多く、英文であることも相まって慣れない文章の作成で時間がかかってしまいました。
Don't get discouraged - or impatient
メール送信後の心がけが書かれています。
Nuno 氏は数日中に返信してくれることが多く、Jonathan 氏は週末に返信してくれることが多かったように記憶しています。
月曜日にパッチを投げた場合には翌週に返信を確認するようなこともあったので気長に待つことが大切だと感じました。
Include PATCH in the subject
メールのタイトルには [PATCH] を先頭につけます。
これは、git send-email コマンドが自動で行ってくれます。
Sign your work - the Developer's Certificate of Origin
Signed-off-by: タグをつけることの意味が説明されています。
主にパッチの著作権に関する話です。
When to use Acked-by:, Cc:, and Co-developed-by:
Acked-by:, Cc:, Co-developed-by: タグの使い方が説明されています。
今回はこれらのタグを使用することはありませんでした。
Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:
Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: タグの使い方が説明されています。
今回は Reviewed-by: と Fixes: のタグが該当しました。
The canonical patch format
フォーマットについて説明されています。
メールのタイトルや本文の書き方が例を交えて書かれています。
git send-email を使うことで基本的な部分は押さえられます。
主に本文を 70 から 75 文字で行を折り返すように気を付ければ良いでしょう。
-v2, -v3 といったバージョンを更新した際に使うオプションもあるのでパッチを修正しての再投稿も楽です。
--annotate オプションを使えば、--- を追加してエディタを開いてくれるので変更点の要約も簡単に追加できます。
これらのオプションの使い方は、チュートリアルが参考になりました。3
Explicit In-Reply-To headers
新しい投稿から過去の投稿を参照する方法が説明されています。
過去のメールの Message-Id を新しいメールの In-Reply-To ヘッダに指定できます。
今回は単体のパッチでしたが、https://lore.kernel.org へのリンクをメール本文のアノテーションに記述しました。
Providing base tree information
トピックブランチを作成してパッチを作成する方法が説明されています。
パッチの投稿に時間が空くこともありましたが、幸いにも途中で対象となるソースコードの変更はありませんでした。
そのため、この機能を使うことはありませんでした。
おわりに
以上でドキュメントの内容は全部になります。
実際には、最初のパッチを投稿してからアドバイスをもらって何度かバージョンを上げていきました。
今回の執筆にあたり振り返りを行うことで気を付ける点を洗い出すことができました。
1. コミットメッセージはユーザー目線で
最初のパッチではコミットメッセージにデバイスの挙動を記述していましたがよろしくありませんでした。
指摘を受けユーザーから見て具体的に何が起きるのかを説明するように修正しました。
- NG だったコミットメッセージ
ADC using ad7793.ko, such as AD7794, may read incorrect data.
Extra interrupt is pending if the data on DOUT contains a falling edge.
Therefore, wait_for_completion_timeout returns immediately.
This patch fixes the issue by setting IRQ_DISABLE_UNLAZY flag.
- OK だったコミットメッセージ
The Sigma-Delta ADCs supported by this driver can use SDO as an interrupt
line to indicate the completion of a conversion. However, some devices
cannot properly detect the completion of a conversion by an interrupt.
This is for the reason mentioned in the following commit.commit e9849777d0e2 ("genirq: Add flag to force mask in
disable_irq[_nosync]()")A read operation is performed by an extra interrupt before the completion
of a conversion. At this time, the value read from the ADC data register
is the same as the previous conversion result. This patch fixes the issue
by setting IRQ_DISABLE_UNLAZY flag.
2. Subject の subsystem は忘れずに
今回のパッチは drivers/iio/adc/ad_sigma_delta.c に対するものだったので、iio: adc: ad_sigma_delta: となります。
タイトルに subsystem を含めずに投稿してしまいましたが、Jonathan 氏が指摘してくれました。
3. Fixes: のハッシュは 12 桁
当初 Fixes: タグを使うことは考えておらず、git config を設定していませんでした。
[PATCH v5] で Fixes: タグの内容を手打ちしましたが、コミットハッシュが 11 桁ではなく 12 桁ということを失念していました。
あらかじめ設定しておいてコマンドを実行すると良いです。
Jonathan 氏がコミットする際に修正してくれました。
ありがとうございます。
4. レビューをもらったらタグを忘れずに
[PATCH v5] ではパッチのソースコードに変更がないので、Reviewed-by: タグをつけるべきでした。
返信の際はパッチの内容や英語の解釈で頭がいっぱいでタグをつけることを失念していました。
Nuno 氏ごめんなさい。
これらの点を踏まえて、次回以降のパッチ投稿に活かしたいと思います。