All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Jan Kara <jack@suse.cz>
Cc: Ted Tso <tytso@mit.edu>,
	linux-ext4@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 1/3] fs: Create __block_page_mkwrite() helper passing error values back
Date: Tue, 17 May 2011 11:09:03 -0400	[thread overview]
Message-ID: <20110517150903.GA6653@infradead.org> (raw)
In-Reply-To: <1305066574-1573-2-git-send-email-jack@suse.cz>

> +	if (unlikely(ret < 0))
>  		unlock_page(page);
> +	else
>  		ret = VM_FAULT_LOCKED;
>  out:
>  	return ret;

Using two different types of return values here seems rather unclean.
I'd rather use 0 instead of VM_FAULT_LOCKED here, and maybe overload
-EAGAIN for VM_FAULT_NOPAGE.

> +int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
> +		   get_block_t get_block)
> +{
> +	int ret = __block_page_mkwrite(vma, vmf, get_block);
> +
> +	if (unlikely(ret < 0)) {
> +		if (ret == -ENOMEM)
> +			return VM_FAULT_OOM;
> +		/* -ENOSPC, -EIO, etc */
> +		return VM_FAULT_SIGBUS;
> +	}
> +	return ret;

and maybe also add a small inlined block_page_mkwrite_error helper
to translate the values.

Alternatively it might make sense to add a VM_FAULT_ENOSPC return value
so that you could re-use block_page_mkwrite unmodified, and we could
also have better error reporting for that case for the core VM.


  reply	other threads:[~2011-05-17 15:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-10 22:29 [PATCH 0/3] Rewrite ext4_page_mkwrite, fix fs freezing Jan Kara
2011-05-10 22:29 ` [PATCH 1/3] fs: Create __block_page_mkwrite() helper passing error values back Jan Kara
2011-05-17 15:09   ` Christoph Hellwig [this message]
2011-05-18  7:35     ` Jan Kara
2011-05-10 22:29 ` [PATCH 2/3] ext4: Rewrite ext4_page_mkwrite() to return locked page Jan Kara
2011-05-10 22:29 ` [PATCH 3/3] ext4: Block mmapped writes while the fs is frozen Jan Kara
2011-05-17 15:11   ` Christoph Hellwig
2011-05-18  7:56     ` Jan Kara
2011-05-18  8:07       ` Christoph Hellwig
2011-05-18 14:03         ` Eric Sandeen
2011-05-18 15:25           ` Jan Kara
2011-05-18 16:40             ` Eric Sandeen
2011-05-11  0:30 ` [PATCH 0/3] Rewrite ext4_page_mkwrite, fix fs freezing Ted Ts'o
2011-05-11  9:27   ` Jan Kara
2011-05-11 16:48     ` Darrick J. Wong
2011-05-11 17:03       ` Jan Kara
2011-05-11  7:33 ` Amir G.
2011-05-11  9:43   ` Jan Kara
2011-05-11 10:15     ` Amir G.
  -- strict thread matches above, loose matches on Subject: below --
2011-05-18 15:17 [PATCH 0/3] Make block_page_mkwrite() handle frozen fs (V2) Jan Kara
2011-05-18 15:18 ` [PATCH 1/3] fs: Create __block_page_mkwrite() helper passing error values back Jan Kara
2011-05-18 18:07   ` Christoph Hellwig

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=20110517150903.GA6653@infradead.org \
    --to=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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.