All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Subject: Re: [PATCH] filemap: skip range writeback if end offset precedes start
Date: Mon, 14 Nov 2022 12:35:24 -0500	[thread overview]
Message-ID: <Y3J8XPLkSC3LqyhX@bfoster> (raw)
In-Reply-To: <20221107122802.d214dd0a3c546d3a35c71e9c@linux-foundation.org>

On Mon, Nov 07, 2022 at 12:28:02PM -0800, Andrew Morton wrote:
> On Mon, 7 Nov 2022 11:33:46 -0500 Brian Foster <bfoster@redhat.com> wrote:
> 
> > Perhaps a good starting point would be to wrap the check in
> > this patch with a WARN_ON_ONCE() and let it soak in -next for a while?
> > That would avoid excessive noise from repetitive callers [1] but still
> > allow those callsites to be identified/fixed. If there is some really
> > weird fdatawrite-only caller that conflicts, the change could always be
> > loosened up from there (as unlikely as that seems).. Hm?
> 
> Sounds reasonable.
> 
> Please let's be clear in the changelog why we're adding this.  I mean,
> we could add a zillion checks everywhere for misbehaving callers.  Why
> choose this one place in particular?
> 

Ok, so TLDR.. this patch is broken as is. I was probably thinking the
generic_fadvise() case of end == -1 was cast to unsigned by the caller,
but testing shows that is not the case and -1 is passed down as a valid
input. This obviously conflicts with the check as proposed here.

I suppose it might make some sense to drop the analogous check from
__filemap_fdatawait_range() so underlying write/wait behavior is
consistent, and perhaps consider adding a higher level check in the
write_and_wait_range() wrappers. I'd have to make a pass through some of
the callers and think about that some more. I.e.,
filemap_write_and_wait_range() documents that end = -1 is acceptable,
fdatawrite presumably does the right thing in that case, fdatawait skips
when end_byte < start_byte, and most callers seem to actually use
LLONG_MAX anyways.

Brian



      reply	other threads:[~2022-11-14 17:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-28 12:54 [PATCH] filemap: skip range writeback if end offset precedes start Brian Foster
2022-10-31  4:51 ` Andrew Morton
2022-11-07 16:33   ` Brian Foster
2022-11-07 20:28     ` Andrew Morton
2022-11-14 17:35       ` Brian Foster [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y3J8XPLkSC3LqyhX@bfoster \
    --to=bfoster@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.