linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Nick Piggin <npiggin@suse.de>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>, Jan Kara <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: Re: [patch 4/5] ext2: convert to use the new truncate convention
Date: Wed, 9 Dec 2009 15:57:13 +0100	[thread overview]
Message-ID: <20091209145713.GD7044@quack.suse.cz> (raw)
In-Reply-To: <20091208084209.GD19823@wotan.suse.de>

On Tue 08-12-09 09:42:09, Nick Piggin wrote:
> I also have commented a possible bug in existing ext2 code, marked with XXX.
> 
> Cc: linux-ext4@vger.kernel.org
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Nick Piggin <npiggin@suse.de>
...
> @@ -752,8 +764,8 @@ int __ext2_write_begin(struct file *file
>  		loff_t pos, unsigned len, unsigned flags,
>  		struct page **pagep, void **fsdata)
>  {
> -	return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
> -							ext2_get_block);
> +	return block_write_begin_newtrunc(file, mapping, pos, len, flags,
> +					pagep, fsdata, ext2_get_block);
>  }
  OK, but you should update the code in dir.c using __ext2_write_begin,
shouldn't you?

> +static int ext2_write_end(struct file *file, struct address_space *mapping,
> +			loff_t pos, unsigned len, unsigned copied,
> +			struct page *page, void *fsdata)
> +{
> +	int ret;
> +
> +	ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
> +	if (ret < len)
> +		ext2_write_failed(mapping, pos + len);
> +	return ret;
>  }
  OK, when doing this, please also update ext2_commit_chunk in dir.c...

> +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
> +{
> +	/*
> +	 * XXX: it seems like a bug here that we don't allow
> +	 * IS_APPEND inode to have blocks-past-i_size trimmed off.
> +	 * review and fix this.
> +	 *
> +	 * Also would be nice to be able to handle IO errors and such,
> +	 * but that's probably too much to ask.
> +	 */
> +	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> +	    S_ISLNK(inode->i_mode)))
> +		return;
> +	if (ext2_inode_is_fast_symlink(inode))
> +		return;
> +	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> +		return;
  Yes, I'd remove IS_APPEND check from here.

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

  reply	other threads:[~2009-12-09 14:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-08  8:38 [patch 1/5] fs: truncate introduce new sequence Nick Piggin
2009-12-08  8:39 ` [patch 2/5] fs: convert simple fs to new truncate Nick Piggin
2009-12-09 14:39   ` Jan Kara
2009-12-10  5:15     ` Nick Piggin
2009-12-10  8:44       ` Boaz Harrosh
2009-12-10  7:57         ` Nick Piggin
2009-12-08  8:41 ` [patch 3/5] tmpfs: convert to use the new truncate convention Nick Piggin
2009-12-09 14:45   ` Jan Kara
2009-12-10  0:48     ` Nick Piggin
2009-12-08  8:42 ` [patch 4/5] ext2: " Nick Piggin
2009-12-09 14:57   ` Jan Kara [this message]
2009-12-10  1:11     ` Nick Piggin
2009-12-10 10:20       ` Jan Kara
2009-12-08  8:43 ` [patch 5/5] fat: " Nick Piggin
2009-12-09 15:08   ` Jan Kara
2009-12-09 14:38 ` [patch 1/5] fs: truncate introduce new sequence 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=20091209145713.GD7044@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=hch@infradead.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=npiggin@suse.de \
    --cc=viro@ZenIV.linux.org.uk \
    /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 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).