tip tree handbook

Tip tree의 역할과 subsystem

Tip tree는 여러 subsystem과 development area의 집합이다. 직접 개발하는 tree인 동시에 여러 sub-maintainer tree를 모으는 aggregation tree다.

• Tip tree git repositoryhttps://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git

x86 architecture

x86 KVM과 XEN 전용 부분을 제외한 x86 architecture development는 tip tree에서 이루어진다. KVM과 XEN 부분은 각 subsystem이 관리해 mainline으로 직접 보낸다. 그래도 x86-specific KVM·XEN patch에는 x86 maintainer를 CC하는 것이 좋은 practice다.

일부 x86 subsystem에는 전체 x86 maintainer 외에 별도 maintainer가 있다. MAINTAINERS가 전체 x86 maintainer를 표시하지 않더라도 arch/x86 file을 건드리는 patch에는 이들을 CC한다.

[email protected]는 mailing list가 아니라 x86 top-level maintainer team으로 전달하는 mail alias다. LKML [email protected]를 항상 CC해야 한다. 그렇지 않으면 mail이 maintainer의 private inbox에만 들어간다.

Core subsystem branch

대상 tree·branch와 patch subject

일반적으로 tip tree master head를 기준으로 개발해도 된다. 그러나 별도로 관리되고 자체 git tree를 가지며 tip에는 aggregation만 되는 subsystem은 해당 subsystem tree 또는 branch를 기준으로 개발한다.

Mainline을 대상으로 하는 bug fix는 항상 mainline kernel tree에 적용 가능해야 한다. Tip tree에 이미 queue된 change와의 conflict는 maintainer가 처리한다.

Tip tree가 선호하는 subject prefix는 subsys/component: 형식이다. 예를 들면 x86/apic:, x86/mm/fault:, sched/fair:, genirq/core:다. File name이나 전체 path를 prefix로 쓰지 않는다. 대부분 git log path/to/file에서 적절한 hint를 찾을 수 있다.

Subject line의 압축된 patch description은 uppercase letter로 시작하고 imperative tone으로 쓴다.

Changelog 구조와 표현

Submitting patches guide의 changelog 일반 규칙을 따른다. Tip maintainer는 imperative mood로 쓰고 code나 code execution을 의인화하지 말라는 규칙을 특히 중시한다. 추상적이고 직접적인 표현은 소설처럼 쓴 문장보다 정확하고 덜 혼란스럽다.

모든 내용을 한 paragraph에 몰지 말고 context, problem, solution을 각각 분리해 이 순서로 쓴다.

예 1: CPU hotplug의 MBM overflow

원래 subject “x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu”는 dying CPU에서 worker를 cancel하고 같은 domain의 다른 CPU에 새 worker를 schedule하지만 timer가 0.99초처럼 expire 직전이면 interval이 사실상 두 배가 된다고 설명한다. 해결 문장은 dying CPU의 delayed work를 cancel하고 다른 CPU에서 즉시 worker를 실행하며, worker가 같은 CPU에 자신을 reschedule하고 domain->cpu_mask를 scan하므로 flush하지 않는다고 길게 서술한다.

개선본 subject는 “x86/intel_rdt/mbm: Fix MBM overflow handler during CPU hotplug”다. Context와 problem을 나눠 expire 직전 reschedule이 interval을 두 배로 만들어 overflow를 놓칠 수 있다고 분명히 한다. Solution은 worker를 cancel하고 같은 domain의 다른 CPU에 즉시 reschedule한다고 명령형으로 쓴다. Flush도 가능하지만 같은 CPU에 reschedule된다는 tradeoff를 덧붙인다.

예 2: POSIX CPU timer

원문은 cpu_timer_sample_group이 -EINVAL을 반환하면 *sample을 쓰지 않으므로 return value를 검사해 now의 uninitialized use를 막고 invalid clock_idx가 *oldval을 undefined하게 덮어쓰는 일을 예방한다고 서술한다. && short-circuit로 결과가 실제 사용될 때만 timer를 sample한다고도 말한다.

개선본 subject는 “posix-cpu-timers: Make set_process_cpu_timer() more robust”다. Return value를 검사하지 않아 compiler와 static checker가 now의 uninitialized use를 정당하게 경고하지만 모든 call site가 valid clock ID를 넘겨 runtime issue는 아니라고 context를 바로잡는다. *oldval이 NULL이라 결과를 쓰지 않을 때도 cpu_timer_sample_group()을 무조건 호출한다는 별도 문제를 적고, invocation을 conditional하게 만들고 return value를 검사하라고 solution을 간결히 제시한다.

예 3: imperative mood

“The entity can also be used for other purposes. Let's rename it to be more generic.”보다 “The entity can also be used for other purposes. Rename it to be more generic.”처럼 직접적인 명령형을 사용한다.

Race와 deadlock을 changelog에서 시각화하기

복잡한 race condition과 memory ordering issue는 CPU별 parallelism과 event의 시간 순서를 table로 보여 주면 가치가 크다.

Lockdep output도 possible deadlock scenario를 같은 방식으로 유용하게 보여 준다.

Function 표기와 commit tag 순서

Function reference

Subject나 changelog 본문에서 function을 언급할 때는 function_name() 형식을 사용한다. 괄호를 생략하면 variable인지 function인지 모호하다. “Make reservation_count static”보다 “Make reservation_count() static”, “reservation_count is only used in reservation_stats”보다 “reservation_count() is only called from reservation_stats()”가 정확하다. Backtrace는 backtraces 지침을 따른다.

Fixes tag

첫 tag는 Fixes: 12+char-SHA1 ("sub/sys: Original subject line") 형식이다. Stable backport가 필요 없는 tip 또는 current mainline head의 최근 issue에도 붙인다. Machine이 자동 추출할 수 있어 culprit commit을 changelog 본문 첫머리에서 강조하는 것보다 유용하다.

The recent replacement of foo with bar left an unused instance of
variable foo around. Remove it.

Fixes: abcdef012345678 ("x86/xxx: Replace foo with bar")
Signed-off-by: J.Dev <j.dev@mail>

이 형식은 original commit이 아니라 현재 patch의 문제와 해결을 중심에 두고, culprit reference를 metadata로 보충한다.

전체 tag ordering

• Fixes: 12+char-SHA1 ("subject")
• Reported-by: Reporter <reporter@mail>
• Closes: bug report URL 또는 Message-ID
• Originally-by: Original author <original-author@mail>
• Suggested-by: Suggester <suggester@mail>
• Co-developed-by: Co-author <co-author@mail>와 바로 이어지는 같은 사람의 Signed-off-by pair
• Signed-off-by: Author <author@mail>
• Signed-off-by: Patch handler <handler@mail>
• Tested-by: Tester <tester@mail>
• Reviewed-by: Reviewer <reviewer@mail>
• Acked-by: Acker <acker@mail>
• Cc: cc-ed-person <person@mail>
• Link: 관련 정보 URL

마지막 Co-developed-by/SOB pair 뒤의 첫 Signed-off-by가 git author의 SOB다. 이후 SOB는 개발에 참여하지 않고 patch를 운반한 handler의 실제 경로를 나타낸다. Single author라면 첫 SOB가 primary authorship을 뜻한다. Ack는 Acked-by, review approval은 Reviewed-by로 쓴다.

Handler가 patch 또는 changelog를 수정했다면 changelog 본문 뒤, 모든 tag 앞에 다음 notice를 넣는다. Changelog와 notice 사이, notice와 tag 사이에 empty line이 각각 있어야 한다.

... changelog text ends.

[ handler: Replaced foo by bar and updated changelog ]

First-tag: .....

Handler가 author 대신 mailing list에 patch를 보낸다면 changelog 첫 line에 From: Author <author@mail>를 쓰고 empty line 뒤 본문을 시작한다. 누락하면 sender가 author로 기록된다. Apply 시 From: line은 final git changelog에서 제거되고 resulting commit의 authorship만 설정한다.

From: Author <author@mail>

Changelog text starts here....

Stable backport 대상은 Cc: [email protected] tag를 넣되 mail 발송 때 stable address를 실제 CC하지 않는다.

Mailing list email reference는 lore.kernel.org redirector URL을 사용한다. 관련 topic, patchset, discussion을 가리킨다. 본문에 [1], [2]를 두고 대응 Link trailer 뒤 comment로 번호를 붙일 수 있다.

A similar approach was attempted before as part of a different
effort [1], but the initial implementation caused too many
regressions [2], so it was backed out and reimplemented.

Link: https://lore.kernel.org/some-msgid@here # [1]
Link: https://bugzilla.example.org/bug/12345  # [2]

Git tree에 apply한 original patch submission을 가리킬 때는 automated tool이 retrieval link를 식별하도록 lore.kernel.org 대신 patch.msgid.link domain을 쓴다.

Link: https://patch.msgid.link/patch-source-message-id@here

Reported-and-tested-by 같은 combined tag는 automated extraction을 복잡하게 하므로 사용하지 않는다.

Documentation link, resend와 merge window

Changelog에 documentation link를 제공하면 나중 debug와 analysis에 큰 도움이 된다. 하지만 company website가 재구성되며 URL이 빨리 깨지는 경우가 많다. Intel SDM과 AMD APM은 상대적으로 non-volatile한 예외다.

변동 가능성이 큰 document는 kernel Bugzilla에 entry를 만들고 document copy를 attachment로 올린 뒤 changelog에 그 Bugzilla URL을 제공한다. Patch resend와 reminder는 resend_reminders 지침을 따른다.

Merge window 전후에는 tip maintainer가 patch를 review하거나 merge할 것이라고 기대하지 않는다. Urgent fix를 제외하고 tree가 닫히며 merge window 종료와 새 -rc1 release 뒤 다시 열린다.

Large series는 merge window 시작 최소 1주 전에 merge 가능한 상태로 제출한다. Bug fix, 새 hardware의 작은 standalone driver, minimally invasive hardware enablement patch는 때때로 예외가 된다.

Merge window 동안 maintainer는 upstream change 추적, merge fallout 수정, bug fix 수집과 휴식에 집중한다. Urgent branch는 각 release stabilization phase에 mainline으로 merge된다.

• Kernel Bugzillahttps://bugzilla.kernel.org

Git pull request와 test 기준

Tip maintainer는 tip tree aggregation용 subsystem change를 제공하는 maintainer의 git pull request를 받는다.

새 patch submission을 pull request로 보내는 것은 보통 받지 않으며 mailing list의 올바른 patch submission을 대체하지 않는다. Review workflow가 email 기반이기 때문이다.

큰 patch series라면 test하려는 사람이 쉽게 pull할 수 있도록 private repository의 git branch를 제공하면 좋다. 보통 cover letter에 git URL을 적는다.

Tip maintainer에게 보내기 전에 code를 test한다. 사소한 change가 아니라면 comprehensive하고 무거운 kernel debugging option을 켜서 build, boot, test해야 한다.

Debug option은 kernel/configs/x86_debug.config에 있으며 기존 kernel config에 다음 명령으로 추가한다. 일부 option은 x86-specific이므로 다른 architecture test에서는 뺄 수 있다.

make x86_debug.config

Comment style과 kernel-doc

Comment 문장은 uppercase letter로 시작한다. Single-line comment는 /* This is a single line comment */ 형식이다. Multi-line comment는 opening과 closing delimiter를 독립 line에 두고 각 line 앞에 *를 맞춘다. 긴 comment는 paragraph로 나눈다.

/* This is a single line comment */

/*
 * This is a properly formatted
 * multi-line comment.
 *
 * Larger multi-line comments should be split into paragraphs.
 */

Tail comment는 거의 모든 context, 특히 code의 reading flow를 방해하므로 피한다.

/* This condition is not obvious without a comment */
if (somecondition_is_true) {
        /* This really needs to be documented */
        dostuff();
}

/* This magic initialization needs a comment. Maybe not? */
seed = MAGIC_CONSTANT;

Header의 struct를 compact하고 읽기 좋게 문서화할 때는 예외적으로 C++ style tail comment를 사용할 수 있다. Bitfield 옆에 의미와 reserved field를 정렬하면 각 member 위에 긴 C comment를 두는 것보다 읽기 쉽다.

// eax
u32     x2apic_shift    :  5, // Number of bits to shift APIC ID right
                                  // for the topology ID at the next level
                            : 27; // Reserved
// ebx
u32     num_processors  : 16, // Number of processors at current level
                            : 16; // Reserved

Obvious operation을 comment로 다시 설명하면 distraction이 된다. refcount_dec_and_test()가 refcount를 감소시키고 zero를 검사한다는 말 대신, zero 뒤 수행되는 비직관적 magic의 이유, ordering·locking constraint와 마지막이어야 하는 operation의 이유를 설명한다.

Function과 argument는 free-form comment가 아니라 kernel-doc format으로 문서화한다.

/**
 * magic_function - Do lots of magic stuff
 * @magic:      Pointer to the magic data to operate on
 * @offset:     Offset in the data array of @magic
 *
 * Deep explanation of mysterious things done with @magic along
 * with documentation of the return values.
 */

특히 globally visible function과 public header의 inline function에 적용한다. 짧은 설명만 필요한 모든 static function에 kernel-doc을 쓰면 과할 수 있고 descriptive function name이 tiny comment를 대신하기도 한다. 상황에 맞게 판단한다.

Locking annotation과 bracket rule

Locking requirement 문서화는 좋지만 comment가 최선은 아니다. “Caller must hold foo->lock”이라고만 쓰지 말고 function 안에 lockdep_assert_held(&foo->lock)을 둔다. PROVE_LOCKING kernel은 caller가 lock을 보유하지 않으면 warning을 내지만 comment는 그렇게 할 수 없다.

void func(struct foo *foo)
{
        lockdep_assert_held(&foo->lock);
        ...
}

if, for, while 뒤 statement가 정말 single line일 때만 bracket을 생략한다.

if (foo)
        do_something();

C grammar상 bracket이 없어도 되는 nested control flow는 single-line statement로 보지 않는다. Outer loop에 bracket을 추가하면 reading flow가 좋아진다.

for (i = 0; i < end; i++) {
        if (foo[i])
                do_something(foo[i]);
}

Variable declaration 배치

Function 시작의 variable declaration은 reverse fir tree order, 즉 긴 type·declaration에서 짧은 것으로 내려가는 순서를 선호한다. 반대 순서나 random ordering보다 빠르게 읽힌다.

struct long_struct_name *descriptive_name;
unsigned long foo, bar;
unsigned int tmp;
int ret;

같은 type의 variable은 screen space를 낭비하지 않도록 한 line에 모은다.

unsigned long a, b, c, d;

Variable declaration 자체를 여러 line으로 split하지 않는다. 긴 initialization은 declaration block 뒤 별도 statement로 옮긴다.

struct long_struct_name *descriptive_name;
struct foobar foo;

descriptive_name = container_of(bar, struct long_struct_name, member);

Hardware type과 constant

Hardware를 묘사하거나 hardware access function argument인 variable에는 올바른 u8, u16, u32, u64 type을 사용한다. Bit width가 명확해 truncation, expansion, 32/64-bit 혼동을 피한다.

unsigned long을 쓰면 32-bit kernel에서 모호한 code에도 u64를 권장한다. unsigned long long도 가능하지만 u64가 짧고 target CPU와 무관하게 64-bit width가 필요함을 명확히 보여 준다. unsigned 대신 unsigned int를 쓴다.

Code와 initializer에 literal hexadecimal 또는 decimal number를 직접 쓰지 않는다. Descriptive name의 define을 만들거나 enum을 검토한다.

Struct declaration·documentation·initializer

Struct member name은 tabular하게 정렬한다.

struct bar_order {
        unsigned int    guest_id;
        int             ordered_item;
        struct menu     *menu;
};

Declaration 안에서 member를 comment로 설명하면 format이 이상해지고 member가 가려지는 일이 많으므로 피한다. 대신 struct 앞 kernel-doc comment를 사용하면 읽기 쉽고 kernel documentation에도 포함된다.

/**
 * struct bar_order - Description of a bar order
 * @guest_id:          Unique guest id
 * @ordered_item:      The item number from the menu
 * @menu:              Pointer to the menu from which the item was ordered
 *
 * Supplementary information for using the struct.
 */
struct bar_order {
        unsigned int    guest_id;
        int             ordered_item;
        struct menu     *menu;
};

Static struct initializer는 C99 designated initializer를 쓰고 tabular하게 정렬한다.

static struct foo statfoo = {
        .a              = 0,
        .plain_integer  = CONSTANT_DEFINE_OR_ENUM,
        .bar            = &statbar,
};

C99는 마지막 comma 생략을 허용하지만 마지막 line에도 comma를 권장한다. 나중에 reorder하거나 line을 추가하기 쉽고 future patch도 조금 더 읽기 좋아진다.

Line break와 symbol namespace

Line length를 무조건 80 character로 제한하면 깊게 indent된 code가 읽기 어려워진다. Excessive line break 대신 helper function으로 code를 분리할 수 있는지 검토한다.

80-character rule은 엄격한 규칙이 아니므로 line을 나눌 때 상식을 적용한다. Format string은 절대로 split하지 않는다.

Function declaration이나 call을 나누면 둘째 line의 첫 argument를 첫 line의 첫 argument 위치에 맞춘다.

static int long_function_name(struct foobar *barfoo, unsigned int id,
                              unsigned int offset)
{
        if (!id) {
                ret = longer_function_name(barfoo, DEFAULT_BARFOO_ID,
                                           offset);

Function과 variable namespace는 readability와 grep 가능성을 높인다. Globally visible function·variable과 inline 이름에 subsystem과 component를 합친 x86_comp_, sched_, irq_, mutex_ 같은 string prefix를 붙인다.

Globally visible driver template에 즉시 들어가는 static file-scope function도 backtrace readability를 위해 좋은 prefix를 쓴다. Local static function과 variable은 생략할 수 있다. 다른 local function만 호출하는 진짜 local function은 짧고 descriptive한 이름을 쓸 수 있다.

Vendor-specific file의 static function에 xxx_vendor_나 vendor_xxx_ prefix를 붙이는 것은 도움이 되지 않는다. File 자체로 vendor-specific임이 분명하고 vendor name은 진짜 vendor-specific functionality에만 써야 한다. 일관성과 readability를 목표로 판단한다.

Commit notification bot

Bot이 tip tree의 새 commit을 감시한다. 새 commit마다 [email protected] 전용 list로 email을 보내고 commit tag에 언급된 모든 사람을 CC한다.

Tag list 끝의 Link tag에서 email Message-ID를 가져와 In-Reply-To header를 설정하므로 notification이 original patch submission thread에 올바르게 연결된다.

Tip maintainer와 submaintainer는 merge할 때 submitter에게 답하려 하지만 잊거나 당시 workflow에 맞지 않을 수 있다. Bot message는 기계적으로 생성되지만 “감사합니다. 적용했습니다”라는 의미도 담는다.