Re: [PATCH v3] UDF: Add support for O_DIRECT

[Jan Kara <jack@suse.cz>]

On Tue 04-09-12 16:11:32, Ian Abbott wrote:
> On 2012-09-04 15:39, Jan Kara wrote:
> >   Hello,
> >
> >   first, you have my address wrong (you had suze instead of suse) which is
> >why I wasn't getting your email and not replying (missed the patch in LKML
> >traffic). Second, it's good to CC also linux-fsdevel for UDF related
> >matters (I tend to use that for UDF announcements etc. so people caring
> >about UDF can watch there and don't have to read high-volume LKML).
> 
> Oops, sorry about the misspelling.  Also, I've noted the
> linux-fsdevel for future (I was just following what it said in
> MAINTAINERS).
  I see. Actually I thought linux-fsdevel is inherited from the default of fs/
directory. But now I tried get_maintainer.pl script and I can see that it's
not. I'll update MAINTAINERS.

> >On Tue 04-09-12 10:49:39, Ian Abbott wrote:
> >>Add support for the O_DIRECT flag.  There are two cases to deal with:
> >   Out of curiosity, do you have a use for this feature or is it mostly
> >academic interest?
> 
> I'm planning to use it for an embedded project that needs to stream
> large files off a CompactFlash card, but the data doesn't need to be
> in the buffer cache as its only read once, and the system has very
> limited memory bandwidth so I can't afford the the extra copy.  The
> old version of this project only supported FAT, but that limited the
> file size to about 4GiB.  The filesystem needs to be something
> reasonably Windows-friendly, at least for adding the files to the
> CompactFlash card in the first place.
  OK, that sounds reasonably.

> >>1. Small files stored in the ICB (inode control block?): just return 0
> >>from the new udf_adinicb_direct_IO() handler to fall back to buffered
> >>I/O.  For direct writes, there is a "gotcha" to deal with when
> >>generic_file_direct_write() in mm/filemap.c invalidates the pages.  In
> >>the udf_adinicb_writepage() handler, only part of the page data will be
> >>valid and the rest will be zeroed out, so only copy the valid part into
> >>the ICB.  (This is actually a bit inefficient as udf_adinicb_write_end()
> >>will have already copied the data into the ICB once, but it's pretty
> >>likely that the file will grow to the point where its data can no longer
> >>be stored in the ICB and will be moved to a different area of the file
> >>system.  At that point, a different direct_IO handler will be used - see
> >>below.)
> >   Sorry, I didn't quite get this. What is the problem with copying all the
> >data to inode in udf_adinicb_writepage() as it is now?
> 
> Part of the good data in the ICB outside the range being addressed
> would get overwritten by zeroes.  This can be tested by creating a
> UDF filesystem with 4KiB blocks and with small files stored in the
> ICB, backed by a block device with 512 byte sectors.  Create a 2KiB
> file with random (or non-zero) data on the file system so that its
> data gets stored in the ICB.  Then open the file for writing without
> truncation and with the O_DIRECT flag set, write 512 bytes at some
> 512 byte offset within the 2KiB file and close it.  If you then
> hexdump the file, you'll find some of the old random data has been
> zeroed out.
  But don't you fall back to buffered IO for files in ICB? So then no
zeroing should happen?

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR