[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHk-=whFKL4VuFBWvenG8fAgfvbf36PDgouUSx47rZDWr9BkJw@mail.gmail.com>
Date: Fri, 24 Feb 2023 08:06:16 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: David Howells <dhowells@...hat.com>
Cc: Steve French <stfrench@...rosoft.com>,
Vishal Moola <vishal.moola@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Jan Kara <jack@...e.cz>, Paulo Alcantara <pc@....nz>,
Matthew Wilcox <willy@...radead.org>,
Huang Ying <ying.huang@...el.com>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
Xin Hao <xhao@...ux.alibaba.com>, linux-mm@...ck.org,
mm-commits@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH] cifs: Fix cifs_writepages_region()
On Fri, Feb 24, 2023 at 6:31 AM David Howells <dhowells@...hat.com> wrote:
>
> Here's the simplest fix for cifs_writepages_region() that gets it to work.
Hmm. The commit message for this is wrong.
> Fix the cifs_writepages_region() to just skip over members of the batch that
> have been cleaned up rather than retrying them.
It never retried them. The "skip_write" code did that same
start += folio_size(folio);
continue;
that your patch does, but it *also* had that
if (skips >= 5 || need_resched()) {
thing to just stop writing entirely.
> I'm not entirely sure why it fixes it, though.
Yes. Strange. Because it does the exact same thing as the "Oh, the
trylock worked, but it was still under writeback or fscache" did. I
just merged all the "skip write" cases.
But the code is clearly (a) not working and (b) the whole skip count
and need_resched() logic is a bit strange to begin with.
Can you humor me, and try if just removing that skip count thing
instead? IOW, this attached patch? Because that whole "let's stop
writing if we need to reschedule" sounds truly odd (we have a
cond_resched(), although it's per folio batch, not per-folio), and the
skip count logic doesn't make much sense to me either.
SteveF?
Linus
View attachment "patch.diff" of type "text/x-patch" (875 bytes)
Powered by blists - more mailing lists