linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Ian Abbott <abbotti@mev.co.uk>
Cc: Jan Kara <jack@suse.cz>, Ian Abbott <ian.abbott@mev.co.uk>,
	lkml <linux-kernel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v3] UDF: Add support for O_DIRECT
Date: Wed, 5 Sep 2012 18:00:19 +0200	[thread overview]
Message-ID: <20120905160019.GG18051@quack.suse.cz> (raw)
In-Reply-To: <20120905125533.GD18051@quack.suse.cz>

On Wed 05-09-12 14:55:33, Jan Kara wrote:
> On Wed 05-09-12 14:05:20, Jan Kara wrote:
> > On Tue 04-09-12 16:11:32, Ian Abbott wrote:
> > > >>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?
>   Oh, I've tested things now and the bug is in buffered write as well!
> It has nothing to do with direct IO. We cannot use simple_write_begin() for
> UDF when the file is in ICB. I'll write a proper fix.
  Fix sent. Please resend your patch without that writepage() change which
shouldn't be needed now. Thanks.

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

  reply	other threads:[~2012-09-05 16:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-24 12:17 [PATCH] " Ian Abbott
2012-07-31 10:40 ` [PATCH v2] " Ian Abbott
2012-09-04  9:49   ` [PATCH v3] " Ian Abbott
2012-09-04 14:39     ` Jan Kara
2012-09-04 15:11       ` Ian Abbott
2012-09-05  9:03         ` Ian Abbott
2012-09-05 12:05         ` Jan Kara
2012-09-05 12:55           ` Jan Kara
2012-09-05 16:00             ` Jan Kara [this message]
2012-09-05 16:44               ` [PATCH v4] " Ian Abbott
2012-09-05 19:02                 ` Jan Kara
2012-09-06 10:08                   ` [PATCH] UDF: Fix incorrect error handling in udf_direct_IO() Ian Abbott
2012-09-06 14:22                     ` Jan Kara

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=20120905160019.GG18051@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=abbotti@mev.co.uk \
    --cc=ian.abbott@mev.co.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH v3] UDF: Add support for O_DIRECT' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).