All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Jan Kara <jack@suse.cz>, Ritesh Harjani <riteshh@linux.ibm.com>,
	linux-ext4@vger.kernel.org, adilger.kernel@dilger.ca,
	linux-fsdevel@vger.kernel.org, hch@infradead.org,
	cmaiolino@redhat.com, david@fromorbit.com
Subject: Re: [PATCHv5 3/6] ext4: Move ext4 bmap to use iomap infrastructure.
Date: Fri, 6 Mar 2020 21:32:12 -0500	[thread overview]
Message-ID: <20200307023212.GA7845@mit.edu> (raw)
In-Reply-To: <20200304153745.GG8036@magnolia>

On Wed, Mar 04, 2020 at 07:37:45AM -0800, Darrick J. Wong wrote:
> > > This makes me wonder if you still need the filemap_write_and_wait in the
> > > JDATA case because otherwise the journal flush won't have the effect of
> > > writing all the dirty pagecache back to the filesystem?  OTOH I suppose
> > > the implicit write-and-wait call after we clear JDATA will not be
> > > writing to the journal.
> > > 
> > > Even more weirdly, the FIEMAP code doesn't drop JDATA at all...?
> > 
> > Yeah, it should do that but that's only performance optimization so that we
> > bother with journal flushing only when someone uses block mapping call on
> > a file with journalled dirty data. So you can hardly notice the bug by
> > testing...
> 
> If we ever decide to deprecate FIBMAP officially and push bootloaders to
> use FIEMAP, then we'll have to emulate all the flushing behaviors.  But
> that's something for a separate patch.

This is really only needed for LILO, since I believe this is the only
bootloader which uses the output of FIBMAP to determine the block
number where it will attempt to ***write*** into a data block of a
mounted file system.

I seem to recall either Dave or Christoph ranting at one point that
any program which attempted to write into a mounted file system using
the output of FIEMAP was insane, and we should not be encouraging that
kind of wacko behavior.  :-)

What most bootloaders want is simply the accurate list of block
locations so they can write that into the stage 1 bootloader so it can
read the stage 2 bootloader from the disk.  The reason why we have the
JDATA hack in the bmap code is because LILO will get the block
location, and then try to write config information into that block.
So we are trying to prevent LILO's write of the boot command line from
possibly getting rewritten after a journal replay.  (Of course, no
distribution installer would do something as rude as to just forcibly
rebooting the system without a clean unmount, so this would *never* be
a problem, RIGHT?  :-)

In any case, I'd much rather try to get LILO fixed to do something
sane, rather that move that heavy-ugly JDATA code into FIEMAP, where
it might get triggered unnecessarily by 99.9% of the users who are
doing something not-insane.

							- Ted

  reply	other threads:[~2020-03-07  2:32 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-28  9:26 [PATCHv5 0/6] ext4: bmap & fiemap conversion to use iomap Ritesh Harjani
2020-02-28  9:26 ` [PATCHv5 1/6] ext4: Add IOMAP_F_MERGED for non-extent based mapping Ritesh Harjani
2020-02-28 15:26   ` Darrick J. Wong
2020-03-13 19:52   ` Theodore Y. Ts'o
2020-02-28  9:26 ` [PATCHv5 2/6] ext4: Optimize ext4_ext_precache for 0 depth Ritesh Harjani
2020-03-13 20:05   ` Theodore Y. Ts'o
2020-02-28  9:26 ` [PATCHv5 3/6] ext4: Move ext4 bmap to use iomap infrastructure Ritesh Harjani
2020-02-28 15:25   ` Darrick J. Wong
2020-03-02  8:58     ` Ritesh Harjani
2020-03-03 15:47       ` Darrick J. Wong
2020-03-04 12:42         ` Jan Kara
2020-03-04 15:37           ` Darrick J. Wong
2020-03-07  2:32             ` Theodore Y. Ts'o [this message]
2020-03-06 17:49           ` Ritesh Harjani
2020-03-07  0:51             ` Darrick J. Wong
2020-03-07  5:50               ` Ritesh Harjani
2020-03-13 20:16   ` Theodore Y. Ts'o
2020-02-28  9:26 ` [PATCHv5 4/6] ext4: Make ext4_ind_map_blocks work with fiemap Ritesh Harjani
2020-03-13 20:18   ` Theodore Y. Ts'o
2020-02-28  9:26 ` [PATCHv5 5/6] ext4: Move ext4_fiemap to use iomap framework Ritesh Harjani
2020-02-28 15:21   ` Darrick J. Wong
2020-03-14  3:03   ` Theodore Y. Ts'o
2020-02-28  9:26 ` [PATCHv5 6/6] Documentation: Correct the description of FIEMAP_EXTENT_LAST Ritesh Harjani
2020-02-28 15:20   ` Darrick J. Wong
2020-02-28 15:36   ` Matthew Wilcox
2020-03-02  8:10     ` Ritesh Harjani
2020-03-14  3:48       ` Theodore Y. Ts'o

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=20200307023212.GA7845@mit.edu \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=cmaiolino@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=riteshh@linux.ibm.com \
    /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.