| Message ID | 20230213140812.db63c7146ebc396691594b73@linux-foundation.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [GIT,PULL] hotfixes for 6.2 | expand |
On Mon, Feb 13, 2023 at 2:08 PM Andrew Morton <akpm@linux-foundation.org> wrote: > > Kuan-Ying Lee (1): > mm/gup: add folio to list when folio_isolate_lru() succeed Ugh. I really hate fixes like this. The problem came from mis-understanding the return value of folio_isolate_lru(), and thinking that it was a boolean success/failure thing. It wasn't, it was an integer "success/errno" thing, and the sense of the test was wrong. So the patch is - if (!folio_isolate_lru(folio)) + if (folio_isolate_lru(folio)) continue; but at no point was the code *clarified*. Wouldn't it have been much better to write the new code to be if (folio_isolate_lru(folio) < 0) continue; to actually make it clear that this is a "negative error return check". I've pulled this, but I really think that when somebody notices that we had a silly bug because of a misunderstanding like this, it's not just that the bug should be fixed, the code should also be *clarified* at the same time. Linus
The pull request you sent on Mon, 13 Feb 2023 14:08:12 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm tags/mm-hotfixes-stable-2023-02-13-13-50
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f6feea56f66d34259c4222fa02e8171c4f2673d1
Thank you!
On 2/14/2023 6:19 AM, Linus Torvalds wrote: > On Mon, Feb 13, 2023 at 2:08 PM Andrew Morton <akpm@linux-foundation.org> wrote: >> >> Kuan-Ying Lee (1): >> mm/gup: add folio to list when folio_isolate_lru() succeed > > Ugh. I really hate fixes like this. > > The problem came from mis-understanding the return value of > folio_isolate_lru(), and thinking that it was a boolean > success/failure thing. > > It wasn't, it was an integer "success/errno" thing, and the sense of > the test was wrong. So the patch is > > - if (!folio_isolate_lru(folio)) > + if (folio_isolate_lru(folio)) > continue; > > but at no point was the code *clarified*. > > Wouldn't it have been much better to write the new code to be > > if (folio_isolate_lru(folio) < 0) > continue; > > to actually make it clear that this is a "negative error return check". > > I've pulled this, but I really think that when somebody notices that > we had a silly bug because of a misunderstanding like this, it's not > just that the bug should be fixed, the code should also be *clarified* > at the same time. Yes, agree, I need to check the return value of folio_isolate_lru() every time when looking at the code. I can help to create a patch to make it clear for all users.
Linus, please merge this batch of fixes, thanks. The following changes since commit ac86f547ca1002aec2ef66b9e64d03f45bbbfbb9: mm: memcg: fix NULL pointer in mem_cgroup_track_foreign_dirty_slowpath() (2023-01-31 16:44:10 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm tags/mm-hotfixes-stable-2023-02-13-13-50 for you to fetch changes up to ce4d9a1ea35ac5429e822c4106cb2859d5c71f3e: of: reserved_mem: Have kmemleak ignore dynamically allocated reserved mem (2023-02-09 15:56:51 -0800) ---------------------------------------------------------------- 12 hotfixes, mostly against mm/. Five of these fixes are cc:stable. ---------------------------------------------------------------- Alexander Mikhalitsyn (1): mailmap: add entry for Alexander Mikhalitsyn Andrew Morton (1): revert "squashfs: harden sanity check in squashfs_read_xattr_id_table" Arnd Bergmann (1): mm: extend max struct page size for kmsan Christophe Leroy (1): kasan: fix Oops due to missing calls to kasan_arch_is_ready() Isaac J. Manjarres (1): of: reserved_mem: Have kmemleak ignore dynamically allocated reserved mem Jeff Xie (1): scripts/gdb: fix 'lx-current' for x86 Kefeng Wang (1): mm: hwpoison: support recovery from ksm_might_need_to_copy() Kuan-Ying Lee (1): mm/gup: add folio to list when folio_isolate_lru() succeed Li Lingfeng (1): lib: parser: optimize match_NUMBER apis to use local array Qi Zheng (1): mm: shrinkers: fix deadlock in shrinker debugfs Seth Jenkins (1): aio: fix mremap after fork null-deref Shiyang Ruan (1): fsdax: dax_unshare_iter() should return a valid length .mailmap | 2 ++ drivers/of/of_reserved_mem.c | 3 ++- fs/aio.c | 4 ++++ fs/dax.c | 5 +++-- fs/squashfs/xattr_id.c | 2 +- include/linux/mm.h | 12 +++++++++--- include/linux/shrinker.h | 5 +++-- lib/parser.c | 39 ++++++++++++++++++++------------------- mm/gup.c | 2 +- mm/kasan/common.c | 3 +++ mm/kasan/generic.c | 7 ++++++- mm/kasan/shadow.c | 12 ++++++++++++ mm/ksm.c | 7 +++++-- mm/memory.c | 3 +++ mm/shrinker_debug.c | 13 ++++++++----- mm/swapfile.c | 20 ++++++++++++++------ mm/vmscan.c | 6 +++++- scripts/gdb/linux/cpus.py | 2 +- 18 files changed, 102 insertions(+), 45 deletions(-)