lkml.org 
[lkml]   [2018]   [Jun]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] fs: btrfs: Change return type to vm_fault_t
On Wed, Jun 06, 2018 at 07:54:44PM +0530, Souptick Joarder wrote:
> @@ -9009,7 +9007,7 @@ int btrfs_page_mkwrite(struct vm_fault *vmf)
> unlock_extent_cached(io_tree, page_start, page_end, &cached_state);
>
> out_unlock:
> - if (!ret) {
> + if (!ret2) {
> btrfs_delalloc_release_extents(BTRFS_I(inode), PAGE_SIZE, true);
> sb_end_pagefault(inode->i_sb);
> extent_changeset_free(data_reserved);

9013 return VM_FAULT_LOCKED;
9014 }
9015 unlock_page(page);
9016 out:
9017 btrfs_delalloc_release_extents(BTRFS_I(inode), PAGE_SIZE, (ret != 0));
9018 btrfs_delalloc_release_space(inode, data_reserved, page_start,
9019 reserved_space, (ret != 0));

I've noticed that there's 'ret' used on lines 9017 and 19, comparing to
a raw number. Is this going to be ok once vm_fault_t is it's own type?

There's no corresponding define for 0 among the VM_FAULT_* values, I'd
expect 0 to work interchangeably, similar to the blk_status_t type:

https://elixir.bootlin.com/linux/latest/source/include/linux/blk_types.h#L30

#define BLK_STS_OK 0
#define BLK_STS_NOTSUPP ((__force blk_status_t)1)
#define BLK_STS_TIMEOUT ((__force blk_status_t)2)
#define BLK_STS_NOSPC ((__force blk_status_t)3)
...

Your patch is otherwise ok, I'm just curious if this is something to
watch for once vmfault type is switched.

\
 
 \ /
  Last update: 2018-06-06 17:57    [W:0.024 / U:19.568 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site