linux-kernel - Re: [PATCH v2 1/4] mm/filemap: Add folio_lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Apr 2023 03:16:23 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Doug Anderson <dianders@...omium.org>
Cc:     Mel Gorman <mgorman@...hsingularity.net>,
        Hillf Danton <hdanton@...a.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Yu Zhao <yuzhao@...gle.com>
Subject: Re: [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout()

On Wed, Apr 26, 2023 at 02:39:56PM -0700, Doug Anderson wrote:
> Sure, I'm happy to do it like you say. Do you have any suggestions for
> the similar lock_buffer() case, or are you OK w/ the timeout there?

I'd do that similarly, ie:

+++ b/mm/migrate.c
@@ -691,38 +691,30 @@ EXPORT_SYMBOL(migrate_folio);
 static bool buffer_migrate_lock_buffers(struct buffer_head *head,
                                                        enum migrate_mode mode)
 {
-       struct buffer_head *bh = head;
+       struct buffer_head *failed_bh, *bh = head;
 
-       /* Simple case, sync compaction */
-       if (mode != MIGRATE_ASYNC) {
-               do {
-                       lock_buffer(bh);
-                       bh = bh->b_this_page;
-
-               } while (bh != head);
-
-               return true;
-       }
-
-       /* async case, we cannot block on lock_buffer so use trylock_buffer */
        do {
                if (!trylock_buffer(bh)) {
-                       /*
-                        * We failed to lock the buffer and cannot stall in
-                        * async migration. Release the taken locks
-                        */
-                       struct buffer_head *failed_bh = bh;
-                       bh = head;
-                       while (bh != failed_bh) {
-                               unlock_buffer(bh);
-                               bh = bh->b_this_page;
-                       }
-                       return false;
+                       if (mode == MIGRATE_ASYNC)
+                               goto unlock;
+                       if (mode == MIGRATE_SYNC_LIGHT && !buffer_uptodate(bh))
+                               goto unlock;
+                       lock_buffer(bh);
                }
 
                bh = bh->b_this_page;
        } while (bh != head);
        return true;
+
+unlock:
+       /* Failed to lock the buffer and cannot stall */
+       failed_bh = bh;
+       bh = head;
+       while (bh != failed_bh) {
+               unlock_buffer(bh);
+               bh = bh->b_this_page;
+       }
+       return false;
 }
 
 static int __buffer_migrate_folio(struct address_space *mapping,