[lvc-project] [PATCH net 1/3] net: fealnx: fix teardown order in remove

[Fedor Pchelkin]

On Fri, 25. Sep 10:46, netdev-bot+sashiko at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 2 · Medium: 0 · Low: 0 (1)
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] The commit says unregister_netdev() -> dev_close() ->
>   netdev_close() "deletes the timers ... before the rings are freed".
> - [High] The fix assumes that once unregister_netdev() returns,
>   netdev_close() has stopped the Rx/Tx DMA engines, so…

Ну это вроде то, о чём сообщал во внутренних обсуждениях.  Похоже сашико
даже с разными моделями выдаёт примерно одинаковое.

Только здесь обозначили это проблемой в патчах, т.к. придрались к описанию.
В какой-то степени правы.  (но, на мой взгляд, это всё равно pre-existing
issue, которое не должно стопорить процесс по текущим патчам - как с
этим обстоят дела в netdev, сейчас и будем узнавать в процессе, т.к. вся
ии-история очень резво развивалась в последние месяцы)

> 
> Pre-existing issues:
> - [Low] At this commit, fealnx_init_one() calls pci_enable_device() and
>   pci_set_master() (lines 497/499).

А это вообще я бы мусором назвал, ибо в третьем патче данная проблема
фиксится и не очень ясно, зачем об этом отдельно сообщать.  Может быть
порядок патчей поменять и фикс с pci_disable_device поместить в начало..


Касательно ответов мэйнтейнеров - их реакции показались токсичными,
особенно Andrew Lunn.  Ну сделаем мы этот KISS как он написал, так их же
sashiko-bot и сообщит, что это плохое исправление, захардкоживающее кол-во
попыток пробинга драйвера, и потом они автоматом cr (changes-requested)
поставят у себя в системе для патча.  Если сопровождающие netdev такое
примут, а Andrew Lunn среди них не главный, будет странно.

Комментарии Francois Romieu тоже очень придирчивые.  С рефакторингом раз
на раз не приходится - хотя в этих фикс-патчах проведенный одновременно
рефакторинг, на мой взгляд, выглядит органично, вполне является логической
частью изменений и не есть рефакторинг ради рефакторинга - но видимо в
древние неподдерживаемые драйверы предлагают обходиться очень краткими
фикс-патчами.

Стоит ли делать рефакторинг вообще, выделив его в отдельные патчи, можно
уточнить у Francois.